Migrate the isolation_session backend and SDK to the IsolationSession Preview API - #761
Open
adpa-ms wants to merge 8 commits into
Open
Migrate the isolation_session backend and SDK to the IsolationSession Preview API#761adpa-ms wants to merge 8 commits into
adpa-ms wants to merge 8 commits into
Conversation
…ew API (#592) * Fix isolation_session_bindings build.rs version-check path resolution * Migrate isolation_session backend + SDK to the IsolationSession Preview API The IsolationSession WinRT surface MXC consumes is now frozen as the Preview namespace. Regenerate the Rust bindings against it and reshape the consumers to the reduced, stable API: - bindings: regenerate from the Preview WinMD; add the windows-crate Foundation feature (the Preview surface references IClosable). - backend (manager/policy/state_aware/one_shot): the OS now assigns an opaque agent user name at provision and validates identity/token at the service, so the sandbox id tail is that opaque name. Collapse the local/Entra provision and start paths into single token-carrying calls, drop host-folder sharing and the per-session sizing profile, and reject all filesystem/network/proxy policy at every phase. Entra is carried by the start config's user bundle rather than inferred from the sandbox id. - domain/wire: remove the sizing-profile config id; regenerate the dev schema and the SDK wire types. - probe: advertise isolation-session availability via `wxc-exec --probe` (probes.isolationSessionAvailable) instead of a registry build pin. - SDK: drop filesystem/configurationId from the typed configs and gate the isolation_session method on the probe fact. Retail CI green: fmt, clippy --all-features, build+test with the feature on and off, SDK unit, schema/sdk-types codegen, and config validation. VM end-to-end validation is pending. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rework the isolation_session VM E2E corpus for the Preview API The one-shot and state-aware PowerShell suites and their JSON fixtures asserted behavior the Preview migration removed. Bring them in line with the new backend: - drop the filesystem-sharing, path-filter, sizing-profile (configurationId) and start-identity cross-check tests (and their fixtures); - assert that filesystem policy is now rejected (policy_validation) at provision as well as the post-provision phases; - assert the sandbox id tail is the opaque OS-assigned agent user name rather than a client-minted token; - rework the simultaneous-sandbox and concurrent one-shot tests to use per-sandbox %TEMP% markers / a host ACL grant instead of folder sharing; - add a fixture proving an unknown configurationId is gracefully ignored. Verified end-to-end on an isolation-capable VM: one-shot 11/11, state-aware 42/42, SDK node integration 2/2 (55/55). Config schema validation green (157 configs). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refresh isolation_session docs and comments after the Preview migration Post-migration review found documentation and comment staleness left behind by the Preview migration (no functional defects). Bring the prose in line with the shipped backend: - rename the three isolation-session docs to drop the "initial-plan" framing (now living specs): initial-bringup-plan -> oneshot and state-aware-{rust,typescript}-initial-plan -> state-aware-{rust,typescript}; update all inbound links (copilot-instructions, sdk/README). - correct the stale policy matrix and prose: filesystem policy is now rejected (policy_validation) at every phase; remove the deleted configurationId / v2-interface / UPN-match / registration content; the sandbox id tail is the opaque OS-assigned agent user name. - scrub residual internal names from MXC prose/comments: IsoEnvBroker, IsoSessionApp.dll, and the pre-Preview Windows.AI.IsolationEnvironment namespace -> Windows.AI.IsolationSession.Preview; genericize bringup-era OS-side names (agent-user format, host binary, worker-process interface). - rewrite the Lifecycle E "registration leak" test comments to the per-agent-user isolation rationale (RemoveUserAsync is per user) and disambiguate two identical assert messages. - refresh the stale configurationId sample in a content-agnostic config_parser test to a user bundle. - fix the IsolationSession row in copilot-instructions (filesystem rejected at every phase; drop ShareFolderBatchAsync/IsoSessionApp.dll). Retail CI green: fmt, clippy, build x64+arm64 with the feature on, unit tests feature on (359) and off (397), wxc_host_prep (17, elevated). Re-verified end-to-end on an isolation-capable VM: 55/55 (one-shot 11, state-aware 42, SDK node 2). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Surface agent SID and shared ephemeral workspace from the Preview API The IsolationSession Preview API gained two provision-time getters on `IsoSessionUserResult` (`AgentUserSid`, `EphemeralWorkspacePath`). Regenerate the bindings and surface both as provision metadata: - bindings: regenerate from the newer Preview WinMD; the only surface change is the two additive getters (the `IIsoSessionUserResult` IID recomputes accordingly). No other interface changed. - manager: `add_user` now returns a `ProvisionedUser` carrying the agent user name, the agent SID, and the shared ephemeral workspace path (read from the three `IsoSessionUserResult` getters). - state-aware: extend `IsolationSessionProvisionMetadata` with `agentUserSid` and `ephemeralWorkspacePath` and populate them at provision. The `sandboxId` tail (the addressing key) is unchanged. - one-shot: adapt the `add_user` call site; one-shot still returns no provision metadata, so it surfaces nothing new. - SDK: add the two fields to the `IsolationSessionProvisionMetadata` type and refresh the unit-test fixtures. The ephemeral workspace is a directory shared between the calling user and the isolated agent user (the caller can stage files into the session through it); each isolated user can access only its own workspace, and it is deleted when the sandbox is deprovisioned. It does not change the workload's working directory. Tests: - Rust unit: provision metadata serializes to exactly the three camelCase wire keys. - VM state-aware E2E (Lifecycle F): metadata presence, caller<->session file sharing, cross-session workspace isolation (a session cannot read a peer's workspace), and workspace deletion on deprovision. - SDK integration: asserts the new metadata fields are present. Validation: fmt, clippy (all-features), build + unit tests feature on and off, wxc_host_prep (elevated), SDK unit, schema/sdk-types codegen, config validation -- all green. Clean-room package build (x64 + arm64) green. VM end-to-end on an isolation-capable build: 62/62 (one-shot 11, state-aware 49, SDK node 2); manual TTY operator-confirmed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: adpa-ms <> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…onSession (#683) * Stop the dev schema advertising a stop/deprovision config for IsolationSession `wire::IsolationSession` reused a single `IsolationSessionPhase` for all four per-phase state-aware slots, but the backend's `StatefulSandboxBackend` impl declares `StopConfig`/`DeprovisionConfig`/`ExecConfig` as `()`. The generated dev schema and SDK wire types therefore advertised an optional `user` payload for `stop`/`deprovision` that `deserialize_config` rejects at dispatch. Drop the two fields from the wire model and regenerate both artifacts. The SDK never emitted those slots (it lifts `version` to the envelope top level), so only a hand-authored raw-JSON caller reading the schema could be misled; there is no behavior change. Add two regression tests in the iso backend, where both halves of the contract are visible: one pins the wire model's per-phase key set (built field-by-field, so a newly added field breaks the build instead of silently regenerating), the other pins that the `()` config phases reject a payload. The existing codegen gate only proves the artifacts match the wire model, not that the wire model matches the backend. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f3b1916-3f39-4dcc-a420-2ce5db5898bd * Address review O1/O2: precise exec wording, pin the wire->config direction O1 — the doc comment on `wire::IsolationSession` grouped `exec` with `stop` and `deprovision` as "invoked via the top-level `phase` field with `sandboxId`". That is correct for stop/deprovision and incomplete for exec, which also requires the top-level `process` block: `validate_exec_common` rejects an empty `process.commandLine` as `malformed_request`, and the dev schema root carries no `required` array, so this description is the only in-schema guidance a hand-authored caller gets. Separate the two cases and regenerate both artifacts, since the text is copied verbatim into each. O2 — the parity tests pinned only the negative direction (the advertised key set, and that the `()` phases reject a payload), while the section comment claimed they pinned both halves. Add `phases_with_a_config_accept_the_wire_payload`, which derives its payload from `wire::IsolationSessionPhase` rather than a JSON literal and asserts the user bundle survives into `ProvisionConfig` and `StartConfig`. That closes a real gap rather than restating existing coverage: on the state-aware path the wire model is never constructed — the dispatcher deserializes raw JSON straight into the config types — so the `From<crate::wire::IsolationUser>` compile-time guard protects only the one-shot path. A rename of the wire `user` key would leave both config types (`#[serde(default)]`, no `deny_unknown_fields`) silently dropping the bundle, provisioning a local sandbox for a caller who asked for an Entra one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f3b1916-3f39-4dcc-a420-2ce5db5898bd --------- Co-authored-by: adpa-ms <> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f3b1916-3f39-4dcc-a420-2ce5db5898bd
…rce; require the unrestricted-network acknowledgment (#682) * Refuse dishonest network policy on the IsolationSession backend The IsolationSession container runs on an unrestricted network (outbound open; a process inside can listen on a localhost-reachable port) that MXC cannot filter or deny. Previously the backend accepted the default `Block` network policy, silently affirming a deny it cannot enforce. Now provision (and one-shot, which runs the full lifecycle) accept ONLY the canonical unrestricted-network acknowledgment: network.defaultPolicy=allow + allowLocalNetwork=true, no allowed/blocked hosts, no proxy, default enforcement. Everything else (including an absent policy, which defaults to the unenforceable `Block`) is refused. Post-provision phases reject any supplied network policy (fixed at provision) via a new domain `ExecutionRequest.network_specified` flag set from wire `network` presence in config_parser; an absent policy is inherited. This closes the domain-model blind spot where an explicit default-valued `Block` is indistinguishable from absent. Reworded the network/proxy error messages and added a post-provision "immutable" message. Updated the happy-path iso test configs to the canonical form; post-provision and negative-path configs are intentionally unchanged. Gates: cargo fmt --check; clippy --all-features; cargo test iso ON and OFF (wxc_common + isolation_session_common green, 84 iso + 456 wxc_common); parser network_specified tests; wxc_host_prep 16 passed (elevated). Pre-existing env failures (microvm e2e staging) are identical iso ON/OFF and unrelated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * SDK: require the unrestricted-network acknowledgment at iso provision Mirror the Rust-side IsolationSession network fix in the TypeScript SDK. `IsolationSessionProvisionConfig.network` is now a required field typed as the exact literal `{ defaultPolicy: 'allow'; allowLocalNetwork: true }`, so the caller must explicitly acknowledge that the isolation session container runs on an unrestricted network the backend cannot filter or deny. Any other value (or a wrong-typed one) is a compile error; the post-provision configs intentionally expose no `network` field, so the type system enforces the provision-only rule for SDK callers. Tests: type-level @ts-expect-error assertions (required network; block and allowLocalNetwork=false rejected; network rejected on post-provision configs); provisionSandbox lifts the canonical network to the envelope top level; updated the conformance oracle's LiftedPhaseKey and the integration test's provision calls. npm run build + npm test green (207 tests). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * docs: IsolationSession network policy is honesty-gated, not rejected Update the in-repo docs to match the new behavior: the backend still rejects all filesystem policy (no host-folder-sharing primitive), but the network policy is now honesty-gated. Provision (and one-shot) require the canonical unrestricted-network acknowledgment (network.defaultPolicy=allow + allowLocalNetwork=true, no host rules, no proxy, default enforcement) and refuse anything else, including an absent policy; post-provision phases reject any supplied network policy and inherit an absent one. Touches: copilot-instructions iso backend row; docs/isolation-session/oneshot (policy-validation coverage row) + state-aware-rust (prose + policy matrix) + state-aware-typescript (provision config table gains the required `network` field); sdk/node/README state-aware example now passes the acknowledgment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * test: expand IsolationSession network coverage + fix E2E probe Add end-to-end and integration tests for the new network-acknowledgment behavior, and fix a latent bug the required-network change introduced in the SDK E2E probe. Fix (critical): `probeStateAwareRuntime` (the module-load skip probe in the SDK integration suite) provisioned iso with no config. With `network` now required at provision, that returns `policy_validation`, which the probe rethrows — so on an iso-capable host the whole integration suite would error at load instead of running. The probe now passes the canonical network acknowledgment for iso. New integration tests (sdk/node integration, real wxc-exec): the backend refuses a provision that (a) omits the network acknowledgment, (b) sends a non-canonical network (defaultPolicy=block), or (c) omits allowLocalNetwork — each via an untyped call, proving the runtime guard holds even when a JS caller bypasses the compile-time type. New E2E tests (PowerShell VM suites): one-shot refuses block / allow-without- allowLocalNetwork / allowedHosts; state-aware refuses a non-canonical network at provision and refuses any network policy on the start and exec post-provision phases (even the canonical acknowledgment, since it is immutable post-provision). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * fix: add canonical network ack to iso TTY resize smoke config The IsolationSession TTY resize smoke (run_isolation_session_resize_smoke.ps1) builds its wxc-exec config inline and was the one manual (-Manual-only) test not covered by the automated suite, so it was missed when the network-acknowledgment requirement landed. Without a network block the backend now rejects it with policy_validation. Add the canonical {defaultPolicy:allow, allowLocalNetwork:true} form (matching isolation_session_powershell_interactive.json) so the manual smoke provisions again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * docs: align state-aware design + GA networking docs with iso network policy The cross-backend state-aware design docs and the v2 GA networking scope doc still described IsolationSession as honoring filesystem (not network) at provision, and showed provision examples using network.allowedHosts -- all now rejected by the backend. Align them with shipped behavior and the SDK type: provision honors only the canonical unrestricted-network acknowledgment ({ defaultPolicy: 'allow', allowLocalNetwork: true }); filesystem policy is rejected and ui is ignored. - networking.md: reword the IsolationSession network-scope line (requires the canonical ack; rejects everything else). - mxc-state-aware-sandbox-api.md / -overview.md: fix the 10.3 honor matrix (iso filesystem applied->rejected, ui applied->ignored), the SDK-exposure prose, the IsolationSessionProvisionConfig type (required network literal; drop filesystem and ui), and the provision examples (drop filesystem, allowedHosts->allowLocalNetwork). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * docs: address PR review -- stale validator comment, fragile test counts, policy.ui gap state_aware.rs: the block comment above the validate_<phase> hooks still said network policy was rejected at every phase. That stopped being true when provision started accepting the canonical unrestricted-network acknowledgment. Correct the network clause while keeping the statements that are still true (the backend has no network primitive; proxy policy is rejected at every phase). oneshot.md: drop the Test Plan Count column and the '~31 backend-specific' / '287 total currently passing' figures. They were already stale (policy.rs has 28 tests, listed as ~24) and the rows never summed to the stated total. No other backend doc or README tracks test counts, so removing them makes this doc consistent and deletes a number that silently rots whenever a test is added. The durable Category / Location / What-it-verifies content stays. state-aware-rust.md: policy.ui was documented as rejected at every phase, but the backend never validates it, so a supplied UI policy is silently ignored. Correct the matrix to 'ignored' (matching the runtime and the state-aware design doc) and add an explicit known-gap note -- this is the same false-guarantee shape the network honesty gate closes, so rejecting policy.ui is the intended end state, not a deliberate exemption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb * refactor: move network_specified onto ContainerPolicy Addresses PR review feedback: the presence signal describes the content of the policy object, not the invocation, so it belongs beside the other parse-derived policy member (`#[serde(skip)] network_proxy`) rather than next to ExecutionRequest's invocation flags (experimental_enabled, testing_features_enabled, dry_run, audit). Behavior-preserving: the flag is still captured once in the parser (`convert_wire_config`), which is also the path `mxc_engine::build_request` round-trips through, so the Rust SDK / FFI / C# callers are unaffected. `#[serde(skip)]` keeps it off the wire, so the schema and generated SDK types are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb --------- Co-authored-by: adpa-ms <> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 47662115-cf22-4564-8930-370dfbee63fb
…velope (#708) * feat(iso): surface structured error fields on the state-aware wire envelope Promote the components of an IsolationSession failure out of the concatenated `message` string and into discrete fields on the wire error envelope: `operation`, `nativeCode` and `remediation`, alongside the existing `code` and `message`. On the state-aware path `message` becomes the bare human-readable text; for a semantic API failure that is the API's own message, passed through verbatim. Wire model (`wxc_common::mxc_error`) - `ApiFailure { operation, native_code?, remediation? }`, held boxed on `MxcError`. Grouping makes the envelope invariant unrepresentable to violate -- `nativeCode` and `remediation` cannot exist without `operation` -- and keeps `MxcError` small enough that every `Result<_, MxcError>` in the workspace stays under clippy's `result_large_err` threshold. - `ErrorEnvelope` gains the three fields, each omitted when unset; `native_code` serialises as camelCase `nativeCode`. IsolationSession backend - `Lifecycle`/`Stale` carry the components structurally instead of a pre-formatted string; `LifecycleFailure::Internal` makes an MXC-side failure structurally incapable of naming an API operation. - Classification split into a pure function so the rules are unit-testable -- `IsoSessionError` is WinRT-activated and cannot be constructed in a test. Same split applied to the activation-failure mapping, which now reports `backend_unavailable` with its operation and HRESULT. - `operation` is interface-qualified, low-cardinality and parameter-free (a failing environment insert names the variable in `message`). - Fixes a latent bug: the `ERROR_NOT_FOUND` -> `stale_id` promotion applied to provision too, which cannot produce a stale id because it mints the sandbox. It is now restricted to non-provision operations, and stays semantic-path only -- a transport HRESULT of the same value has none of the provenance that gives it that meaning, so promoting it would emit a false `stale_id` and tell the caller to destroy a healthy sandbox. One-shot is deliberately untouched: `Display` still composes the full human string, including the category prefix, because that path has no structured envelope to read the fields from. TypeScript SDK - `MxcError` gains a constructor overload taking a flat `MxcErrorFields` object mirroring the wire shape. The positional signature is retained and declared last, so existing callers and `ConstructorParameters<typeof MxcError>` are unaffected. - `mxcErrorFromEnvelope` is the single wire-to-error boundary, including the unknown-code passthrough; all envelope-parsing sites route through it. Also removes several pre-existing OS-internal names from prose in the files touched, per repo convention. Verified on the retail host: cargo fmt, clippy (--all-features), build and test with isolation_session ON and OFF, SDK unit, SDK integration, the versioning gate suite, and wxc_host_prep in an elevated shell -- all green. The iso E2E suites still need a VM run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2 * fix(iso): never emit an empty error message; state operation-value stability Addresses both optional findings from the review of #708. O1 -- the wire `message` could be empty. `Message()` is a best-effort WinRT getter, and with the operation and HRESULT now in their own fields nothing backfills `message`, so a failed or empty getter reached the wire as `"message": ""`. The change was internally inconsistent about it: the `Err(Code())` arm already guarded the empty case, and `remediation` normalised empty-to-absent, but the `Ok(code)` arm passed the raw string through. Both best-effort getters now collapse to `Option` at the WinRT boundary and `IsoApiFailure::new` decides what absent means per field -- a stand-in for `message`, which the wire requires, and absence for `remediation`, which is optional. Normalising at construction rather than per branch is what keeps the guarantee from having to be restated at each call site; every construction path routes through it. O2 -- `operation` values are now published in the SDK README, recommended for telemetry aggregation, and pinned by an E2E assertion, but nothing said whether they are stable. They mirror the projected WinRT class and method names, which this repo does not own and cannot version, so they are now documented as best-effort diagnostics rather than a versioned contract, in the cross-backend contract, the backend spec, and the SDK README. The E2E assertion that pins an exact value carries a note explaining why pinning is correct there specifically: it verifies MXC's own mapping and moves with the constant. Also verified the boxing rationale the review could not check without running clippy: `MxcError` is 72 bytes as written and would be 136 inlined, against the default 128-byte `result_large_err` threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2 * fix(errors): address PR review round 2 Fixes found by review of the structured-error-fields change. - transport_err no longer emits a dangling "step: " when the platform supplies no message text. An HRESULT with no OS message-table entry (0xDEADBEEF, and any custom facility code) returns an empty message(), and joining unconditionally produced a technically-non-empty string that slipped past the empty-message guard in IsoApiFailure::new. Fall back to the step alone. ~34 call sites route through this one join. - MxcError::Display now renders the API detail when present, so a consumer that only logs the error keeps the operation and status that used to be concatenated into message. Rendering only: the wire envelope still carries message bare, with the components in their own fields. Replaces the thiserror derive with explicit Display + Error. - The Code()-getter-failure branch moves into unreadable_code_failure, a pure function, so its composition is reachable from a unit test. format_iso_error stays a thin WinRT adapter. - Correct the ApiFailure doc comment: grouping makes the invariant the easy path, not an unrepresentable-to-violate one (Default was derived and the fields are pub). Drop the unused Default derive. - Add #[non_exhaustive] to MxcError and ErrorEnvelope so future fields are a non-event for other workspace crates. - Guard the MxcError constructor against a nullish argument, which took the object branch and failed inside super() with a TypeError naming "message". Default the positional message rather than asserting it. - Un-export WireError and mxcErrorFromEnvelope: they exist so the SDK's own parse sites share one widening point. MxcErrorFields stays exported because it is the parameter type of a public constructor overload -- hiding the name leaves the type usable but unnameable. - Lift the four host-independent policy-validation cases out of the probe-gated suite. Both CI systems set MXC_SKIP_OS_BUILD_DEPENDENT_TESTS=1, so nothing in that suite ran in CI; these need the isolation_session feature compiled in but not a host that can run isolation sessions. - Document that the structured fields are currently populated only by IsolationSession state-aware operations. Gates: fmt; clippy --all-targets --all-features -D warnings; Rust build + test iso ON and iso OFF (wxc_host_prep 16/16 elevated); SDK unit 231/0; SDK integration 45/0 with the four lifted cases now executing under the CI skip flag; versioning + dotnet parity 7/7; VM suites 73 passed / 0 failed with an empty leak delta; manual TTY tests confirmed by the operator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2 --------- Co-authored-by: adpa-ms <> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2
…an dropping them (#718) * docs(isolation-session): correct claims that no longer match the code oneshot.md listed lifecycle.destroyOnExit under Implemented and described it as mapped to an OS-side lifetime policy; the backend has no such knob and silently ignored the field. It also claimed the runner does not use stdin, terminate, control signals or ConPTY -- all four are used -- and carried a stale ConPTY deferral plus pre-Preview-API type names. schema.md omitted the ui section from the Full Schema example and had no ui field table at all, and stated that other backend sections are ignored when validate_single_backend_section rejects them. isolation_session and hyperlight were missing from the concrete-backends table. The cross-backend design doc described IsolationSessionProvisionMetadata as one field when it carries three, and its containerId claim did not match the parser. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c * fix(isolation-session): refuse ui and unsupported lifecycle rather than dropping them A caller could supply a schema-valid ui policy in the documented slot on the correct surface and silently receive none of it. That is the caller doing everything right and still losing a security control they believe is applied, so the backend now refuses it at every phase on both surfaces with policy_validation. The refusal is presence-based via a new ContainerPolicy::ui_specified flag, the twin of network_specified. UiPolicy::default() is full lockdown, so an explicitly-supplied lockdown ui is indistinguishable by value from an absent one; without a presence bit the backend cannot tell 'caller asked for lockdown' from 'caller said nothing'. Measured on a live session before choosing to refuse rather than treat lockdown as vacuously satisfied: window creation, GetDC/GetDeviceCaps/GetSystemMetrics and the session's own clipboard all succeed inside the session; only SendInput is denied. The session isolates the HOST's UI from contained code, but the ui fields are written as capability denial, so accepting them would assert a Win32k attack-surface reduction that is not delivered. lifecycle is refused by value on one-shot, where the defaults do match reality: the backend always stops the session and removes the agent user before returning, which is exactly destroyOnExit=true. Only destroyOnExit=false and preservePolicy=true are refused. State-aware already rejected the whole section. Tests at all three tiers: Rust unit for every phase, absent and lockdown-equivalent cases, the filesystem -> ui -> network precedence and ui_specified on both surfaces; a new e2e_isolation_session_policy.rs whose refusals all happen in validate_* before any OS-side call, so it needs no isolation-capable host and skips cleanly when the feature is off, with an over-rejection guard; and Node integration guards, since ui is reachable from plain JS even though the typed per-phase configs exclude it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c * docs(isolation-session): publish the full policy honor matrix for both surfaces The state-aware design's §10.3 scopes the required honor matrix to filesystem / network / ui and to the state-aware surface only. That narrowness is why the ui and lifecycle gaps went unnoticed, so the matrix now covers every field a caller can express, on one-shot and all five state-aware phases, with per-row notes for the rows that are not a simple accept/reject. Rows that are accepted-and-ignored are documented as such rather than quietly omitted: process on non-exec state-aware phases, and mis-slotted experimental.isolation_session payloads (the flat user spelling on state-aware, a nested provision/start block on one-shot, and a block under a phase that is not the request's own). Each is a caller supplying a documented field in an undocumented position; the result is a local rather than Entra-backed sandbox, which is a capability downgrade that surfaces downstream as an auth failure. Detecting mis-slotted payloads generically is a cross-backend concern and is deliberately not solved here. §10.3's normative list and the cross-backend contract are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c * docs(isolation-session): correct three claims left stale by the policy rescope Two honor-matrix claims described behaviour that existed only while the parser carried the generic mis-slotting rules, which were removed as out of scope. Both are now documented as accepted-and-ignored, matching what the code does: * state-aware-rust.md said the parser refuses a non-exec 'process' section. It does not -- the dispatcher simply reads 'process' only on exec, and nothing runs at the other phases. The matrix rows were updated at rescope but this prose was missed. * oneshot.md said a nested experimental.isolation_session.{provision,start} block is rejected on one-shot. The one-shot mapping reads only the flat 'user', so the nested blocks are ignored. The flat 'user' IS still rejected (validate_runner), so that neighbouring row stands. Both verified by probe against the current parser rather than by inspection, along with every other rejection claim in the two documents. Also removes a stray mid-line carriage return introduced in docs/schema.md, which split a JSON sample line from its trailing comma. The EOL check used until now compared git diff --stat against --ignore-cr-at-eol --stat, which by construction cannot see a CR that is not at end-of-line; a bare-CR scan over every changed file is clean. Documentation only -- no code, no schema, no test changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c * docs(isolation-session): state the ui contract plainly and correct four stale claims Review raised that an omitted ui is accepted while the schema says omission equals full lockdown, and proposed requiring an explicit acknowledgment of the real posture, mirroring the network gate. That direction is unsound: there is no truthful ui posture for this backend to acknowledge. The section states intent about the contained code's relationship to the USER's environment, and was modelled on a process/job boundary where 'the clipboard' and 'the desktop' are the user's. An isolation session is a separate OS session, so the contained code keeps its UI capabilities but cannot reach the host's. disable=true denies capabilities the session grants; disable=false promises a GUI the user can never see; every clipboard value describes a relationship to a clipboard the sandbox cannot touch. Only injection=false is honest, and it cannot be supplied alone because the other fields materialize to defaults that are false. An acknowledgment gate needs a true statement to acknowledge, so unlike network there is none available. Documented accordingly, with the field-by-field table in state-aware-rust.md and the rationale in oneshot.md. schema.md's 'omitted ui equals full lockdown' is qualified as per-backend rather than global, and names IsolationSession. ERR_UI_POLICY no longer advises removing the section as though removal were equivalent -- it states that omission is accepted but applies no restriction. Four stale claims corrected, all falsified by this PR's own retained work rather than by the removed work an earlier sweep looked for: * copilot-instructions claimed policy_validation on both surfaces; one-shot discards the typed variant and emits backend_error with the reason in the message. Documented, not changed -- threading a typed code through ScriptResponse touches every backend's one-shot path. * the honor matrix marked an absent network policy rejected on post-provision phases; validate_post_provision_policy gates on network_specified, so absent is inherited. Row split. * the matrix marked every foreign backend section rejected; a lone foreign experimental section on a non-provision phase is accepted and ignored, because those requests carry no containment to compare against. Row split, stable sections kept as rejected. * manager.rs still said lifecycle.destroyOnExit is silently ignored, which this PR made false. Also fixes the preservePolicy message (the backend requires the canonical network acknowledgment rather than rejecting network policy outright) and adds that acknowledgment to both oneshot.md examples, which were non-runnable -- verified through the real binary: both now validate, and the previous shape is refused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c * docs(models): scope the ui_specified comment to its one consumer Review flagged that 'Used by backends (e.g. IsolationSession)' implies several backends consume this flag when exactly one does, and that the phrasing hides an incomplete story: LXC, Bubblewrap, Seatbelt and Windows Sandbox all still accept and ignore policy.ui, so the Linux and macOS instances of the defect this change fixes remain open. The comment now names IsolationSession as the only consumer today, names the backends that accept-and-ignore, and states the consequence plainly -- the flag being set does not mean a UI policy was honored anywhere, only that the caller supplied one. Comment only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c --------- Co-authored-by: adpa-ms <> Copilot-Session: 35b9aab9-16c9-4897-bcdb-f7188a51175c
…boxId (#746) * refactor(isolation-session): remove the one-shot backend config surface The one-shot IsolationSession path takes no configuration. Its only field, `user`, existed solely because the state-aware `StartConfig` reused the one-shot domain struct -- so one-shot had to reject its own struct's only field at runtime. That rejection was guard code compensating for the deliberately permissive `experimental` block (no `deny_unknown_fields`). Such guards are scaffolding that graduation to the closed stable surface deletes anyway, so ignoring is the correct behaviour and matches every other unrecognised key there. - wire `IsolationSession` loses `user`; it now carries only the state-aware `provision` / `start` nesting. - domain `IsolationSessionConfig` becomes `IsolationSessionStartConfig`, which is what it always was in practice. - `ExperimentalConfig.isolation_session` is deleted outright. Nothing is lost: the multi-backend conflict check reads the *wire* struct (`present_backend_sections` takes `&wire::MxcConfig`), which survives. Caller-visible behaviour change: `experimental.isolation_session.user` on a one-shot request changes from a loud error to being silently ignored. It is unreachable from the typed Node SDK, whose one-shot `ContainerConfig.experimental` exposes only `wslc` and `telemetry`. Tests: the deleted rejection tests are replaced, not dropped -- one asserting the field is now accepted and ignored, and one pinning that a lone `experimental.isolation_session` section still marks a configured backend so the conflict check cannot silently regress. Schema and SDK wire types regenerated (not hand-edited). * fix(isolation-session): trim the UPN consistently at validation and at the OS call `validate_isolation_session_user` trimmed the UPN before its shape check, but provision and start handed the OS the untrimmed value. A padded UPN such as " alice@contoso.com " therefore passed validation and reached the OS with its surrounding spaces intact -- validation and transmission disagreed about what the accepted value was. Extract `os_credentials`, which produces the exact (entraAccountName, wamToken) pair given to the OS, and apply the trim there so the two agree. An absent bundle maps to the local-agent empty pair. The WAM token is deliberately NOT trimmed: it is an opaque bearer credential and trimming could corrupt it. The helper exists because the previous inline `match` offered no seam -- the behaviour could not be asserted without a live OS service. It is now covered by unit tests for the trim, the verbatim token, the absent bundle, and the interior-whitespace case. * refactor(isolation-session): split the wire phase type per phase `wire::IsolationSessionPhase` was shared by provision and start, so the generated schema advertised every per-phase field on both phases regardless of which one actually accepts it. The domain configs and the Node SDK types were already split per phase; only the wire model pooled them. Replace it with `IsolationSessionProvisionPhase` and `IsolationSessionStartPhase`. The JSON keys (`provision`, `start`, `user`) are unchanged, so this is invisible on the wire -- it only makes the generated schema and SDK wire types state truthfully where each field is legal. The SDK conformance oracle is now per-phase rather than a single pooled key set, which is strictly stronger: a field legal only on provision can no longer satisfy it by appearing on the start config. The phases whose Rust associated type is `()` are asserted to expose no backend-specific field at all. Also add a non-vacuity guard to that oracle. Every assertion is of the form `Exclude<A, B> extends never`, which passes trivially if `A` resolves to `never` -- so a mistake in the derivation would have silently disabled the check instead of failing it. The derived key sets are now pinned to their expected contents. Schema and SDK wire types regenerated (not hand-edited). * feat(isolation-session): carry an optional appId inside the sandboxId Accept an optional `appId` on the state-aware provision phase -- the Package Family Name for a packaged application, any string for an unpackaged one -- and carry it inside the returned `sandboxId`. Motivation: future OS API changes will act on the calling application's PFN, and those calls are expected to be spread across lifecycle phases. It is not guaranteed that the OS will propagate a PFN supplied at provision to a session's other calls. MXC holds no cross-phase state (each phase is a fresh process), so the only carrier that survives without the caller re-supplying the value on every phase is the sandboxId itself. Embedding works whether or not the OS ends up retaining it. Nothing consumes appId yet. It is accepted, encoded, and decoded back into an internal struct, deliberately exposed nowhere -- scaffolding for a future OS contract, so adopting it later is not a breaking change. New id format, replacing the plaintext `iso:<agentUserName>` tail: iso:<base64url-nopad( JSON object )> with v1 keys `version`, `agentUserName`, and optional `appId`. Encoded rather than delimited because the parser must know which fields are present without assuming anything about separator characters: the agentUserName is OS-assigned with no charset guarantee, so a delimited form would mis-parse a name containing the delimiter *silently*. The base64url alphabet makes that entire class of bug unrepresentable rather than merely prevented. The envelope is frozen (always base64url of a JSON object; all evolution happens as keys inside). The version gate is one-directional -- a payload from a newer MXC is rejected with a message that says so, since the remediation is "upgrade MXC", not "this id is corrupt" -- and is bumped only for changes an old reader must not silently mishandle. Unknown keys are ignored. appId validation is structural only (no control characters, at most 256 characters). MXC is a pass-through carrier and does not judge what a valid application identity looks like; a PFN grammar check would risk rejecting forms a future OS API accepts. The value is preserved verbatim, and an explicitly empty string is a value distinct from absent -- a future OS API may assign it meaning, so MXC neither collapses the two nor ever synthesizes an empty string the caller did not send. Legacy plaintext ids no longer decode and surface as `malformed_id`. Intended: they refer to OS resources that do not survive the change either. Tests: exhaustive codec unit tests (round-trips, empty-vs-absent distinctness, verbatim preservation, hostile agent-user names containing colons and path separators, determinism, the alphabet property, every decode failure mode, the version gate); provision-hook validation tests; SDK type and envelope tests including a compile-time assertion that appId is rejected at start; and E2E coverage for the round-trip, the empty case, both rejections, legacy ids, and newer-version ids. * fix(isolation-session): address review findings on appId/sandboxId Five rounds of review against the four preceding commits. Grouped by what they fix rather than by the round they surfaced in. Correctness -- the id codec - Restore the non-empty agentUserName invariant. The base format guaranteed it structurally (`!rest.is_empty()` applied to the tail, which WAS the name); the rewrite applied that check to the base64 tail, catching only a bare `iso:`. {"version":1,"agentUserName":""} decoded cleanly and handed an empty string to the OS lifecycle calls, which answer "not found" -- surfacing as stale_id ("re-provision") for a request that was never well-formed. - Re-validate appId on decode. sandboxId is caller-supplied on every post-provision phase, so provision is not the only way a value arrives; the guarantee now holds by value rather than by provenance. Mapped to malformed_id, NOT policy_validation: every other decode failure is malformed_id, a bad id is an id problem, and the phases that consume an id accept no policy for a policy error to belong to. - Decode the id in validate_exec / validate_stop / validate_deprovision. Previously only validate_start decoded, so --dry-run (which stops after validation) reported success for ids the real call rejects. The asymmetry is pre-existing -- all three hooks ignored the id at base -- but this change widens the class of ids that fail, so it is closed here. Documentation -- four false or incomplete claims - The legacy-id justification was wrong three times in succession, each correction exposing the next. It is not true that the referenced resources do not survive the change: the agent user account persists until explicit deprovision. It is not true that the session does not outlive the binary: outliving the process is the premise of the state-aware lifecycle, and nothing in MXC stops a session when the binary is replaced. It is not true that such a sandbox becomes unaddressable through MXC: decode binds nothing to the minting binary, so re-encoding the old agent user name -- which a legacy id carries in the clear -- yields a valid id for the same sandbox, making recovery unconditional rather than contingent on having recorded anything. Also corrected: a session ends at deprovision too, since removing the agent user terminates any session still running under it. - A doc this change edits still claimed one-shot rejects experimental.isolation_session.user; the correction had been applied at one location and missed at the parallel statement 200 lines later. - The appId JSDoc promised `null` as a spelling of absent on a `string` property, so a caller following it got a compile error. Claim removed; the wire-level behaviour is unchanged and documented where it applies. - state-aware-typescript.md enumerated the provision config but omitted appId. Tests -- the recurring defect, and the rule that ends it Four rounds surfaced the same class of flaw: a test that exercises the fixed path without discriminating it from the fix's absence. Each would have passed unchanged against the pre-fix head. - A case titled "every id-consuming phase rejects a legacy id" never issued a dry run -- the harness had no dry-run parameter at all. Added -DryRun to Invoke-StateAware and exercised each phase both ways, plus the missing counterpart: a well-formed id must be ACCEPTED by --dry-run on every phase, or the agreement would be satisfied trivially by refusing everything. - The crafted-appId case spliced a raw U+0007 into the JSON text, which RFC 8259 forbids inside a string, so serde_json rejected it at parse time and the payload never reached validate_app_id. Written as \u0007 so the document is valid and the control character survives into the decoded string, and the message is asserted to name `appId` -- which is what distinguishes validate_app_id running from the parser refusing. An oversized-appId case is added, having no JSON-level analogue and so unable to pass for the wrong reason. - The dry-run tests asserted only exit codes, which are identical either way. They now assert the result envelope, and the exec command prints a marker and exits 1 so a dropped flag is caught three independent ways. - That envelope assertion in turn overclaimed: start/stop/deprovision return metadata: None, rendered as the same {"result":{}} the dry-run short-circuit produces, so it discriminates nothing for those three. The comment is scoped to what it proves, and the real observable is added where it can live -- wxc_common's dispatch tests, whose call-counting StubBackend already pinned dry-run for provision and exec. start, stop and deprovision were simply missing. Dry-run skipping is now pinned for all five phases at the layer where dry_run actually lives, in Rust tests that run in the local review loop rather than only on the VM. Verified by mutation rather than by reasoning: making the Start arm ignore dry_run fails dispatch_start_dry_run_skips_start_call_but_runs_validate, and only that test. The rule going forward is to make the fix's absence produce a failure and then observe it. Harness -- a leak inside the leak discipline The positive dry-run case provisions a real sandbox, and Run-StateAwareTest swallows throws while the suite's finally reclaims only $script:sandboxId, which that case never set. A throw between provision and cleanup therefore leaked an Indefinite-lifetime agent user outside the harness's own leak discipline -- the discipline cited as evidence elsewhere in this review. The id is now reclaimed in a dedicated try/finally, and the cleanup's exit code is asserted rather than discarded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c82ca12-a566-4257-9de7-164be299375e --------- Co-authored-by: adpa-ms <> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c82ca12-a566-4257-9de7-164be299375e
Enterprise support is not ready to ship in main. This removes the Entra `user`
API (`{ upn, wamToken }`) from the isolation_session backend so the main-bound
branch carries no enterprise surface. It is restored on
feature/isolation-session-internal by the commit that follows.
Scope is strictly the `user` API. Everything else on the branch -- the
IsolationSession Preview API migration, `appId` in the structured `sandboxId`,
structured error fields, and the network/UI policy rejection work -- is
untouched.
Rust
- Wire model: drop `IsolationUser`, `IsolationSessionStartPhase`, the `start`
slot on `IsolationSession`, and `user` from the provision phase. Provision is
now the only phase carrying a per-phase wire object, which is what the
generated schema advertises.
- Domain model: drop `IsolationSessionUser` and `IsolationSessionStartConfig`.
The latter's only field was `user`, so the type has nothing left to carry --
matching Rust's existing pattern, where a phase type exists only if the phase
contributes a wire object (exec/stop/deprovision already use `()`). #683 made
exactly this change for stop and deprovision; start now joins them.
- Backend: `type StartConfig = ()`. Remove `os_credentials` and the start-phase
shape validation. `IsoSessionOps.AddUserAsync` / `StartSessionAsync` keep the
OS-defined optional account/token parameters -- MXC now always passes empty
strings, which is what the local-agent path already did -- so the generated
`bindings.rs` is untouched.
- `policy.rs`: remove `validate_isolation_session_user`.
TypeScript
- Remove the `IsolationSessionUserConfig` class (and its `wamToken` inspect
redaction) and the `user` fields on the provision and start configs.
- KEEP `IsolationSessionStartConfig` as `{ version?: string }`. Deleting it
would break the pattern rather than follow it: five sibling interfaces are
already version-only, including `WindowsSandboxStartConfig` -- the same phase
on the other state-aware backend -- and `ConfigsForBackend` requires all five
phase keys per backend. `version` is also not vestigial: state-aware-helper
lifts it out of the backend object onto the envelope as the request's schema
version, so removing the type would leave `start` on isolation_session as the
only (backend, phase) pair a caller cannot version.
Tests
- Wire-conformance: the start-phase equivalence assertions are replaced by
`_StartNoBackendKeys`, joining the existing exec/stop/deprovision group.
Deleting only the failing `_StartKeysNonVacuous` guard and keeping the
equivalences would have left three assertions passing because both sides are
`never` -- vacuously true, which is precisely what that guard exists to catch.
- `phases_without_a_config_reject_a_payload` now covers `start`, pinning that it
moved into the no-config group rather than merely losing a field.
- The SDK integration test "a policy rejection reaches the SDK with no
structured failure fields" is preserved with a different trigger rather than
deleted. It used a malformed UPN only as a vehicle; the contract it pins --
`operation`/`nativeCode`/`remediation` absent when no API call was in flight
-- ships to main with #708 and applies to every policy rejection on every
backend. It is also the only END-TO-END coverage of that contract; the
sibling assertions in `errors.test.ts`, `state-aware.test.ts` and Rust
`error.rs` all use fabricated envelopes. It now triggers on an oversized
`appId` -- the same MXC-side, pre-API-call rejection.
- The two `state_aware_request` secret-redaction tests are DELETED rather than
rewritten. They were written specifically to demonstrate the `wamToken`
path, and every link they covered is pinned elsewhere: `config_deserialize`'s
own self-contained tests already assert redaction on a fully-qualified path
(`experimental.someBackend.user`), and 13 non-secret tests in the same file
cover prefix construction and whole-file line reporting. After this change no
config field in the repo matches a secret marker, so the composition they
exercised is unreachable.
- The one-shot stray-config test is renamed, not dropped: it pins that an
unrecognised `experimental.isolation_session` key is ignored rather than
rejected, which nothing else covers. A key naming nothing real tests that
better than `user` did.
Docs
- `docs/isolation-session/state-aware-rust.md` and `state-aware-typescript.md`
are the authoritative per-backend specs: the provision/start `user` rows, the
`IsolationSessionUserConfig` section, the honor-matrix rows and the Entra
worked example are removed, and Start is restated as taking no per-phase
config.
- `docs/schema.md` loses the now-invalid
`"isolation_session": { "start": { "user": … } }` nesting example. This is
required by the repository convention that a config-field removal updates
`docs/schema.md` alongside the generated schema
(`.github/copilot-instructions.md`). The example was doubly wrong after this
change: `user` no longer exists, and `start` accepts no object at all, so a
caller copying it would get a hard dispatch error rather than a tolerated
unknown key.
- `docs/schema-codegen.md` no longer lists the `user` bundle among the objects
the generated schema closes, and its per-phase nesting list is reduced to
`isolation_session.provision`. (That list was also stale for `stop` /
`deprovision` from #683; the whole line is corrected rather than only the
part this change falsified, since a partial fix would still be wrong.)
- `docs/isolation-session/oneshot.md`, `docs/windows-sandbox/windows-sandbox.md`,
`docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md` and
`.github/copilot-instructions.md` drop their Entra references, including three
instances of the same "WindowsSandbox has no Entra `user` bundle" contrast that
is meaningless once no backend has one.
Deliberately not corrected here: the illustrative
`"start": { "configurationId": … }` examples in
`docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md`. `configurationId`
was already dead before this change and the block is self-caveated as
illustrative. This change does alter *why* those examples are wrong -- the shape
itself is now rejected, not just the field -- but the correct content differs
between this branch and the feature branch that restores the `user` bundle, so a
standalone follow-up lands it once instead of being reverted and re-applied.
`config_deserialize`'s secret-redaction machinery is generic infrastructure and
stays; only its fixtures and the `SECRET_PATH_SEGMENTS` comment are reworded off
the enterprise example. The telemetry threat-model references to UPN are not
enterprise surface -- they document that the correlation-vector base is never
seeded from caller identity -- and are left alone.
The C# SDK is untouched: this branch does not modify `sdk/dotnet`, and it cannot
reach any experimental backend today (`experimental_enabled` is never set on the
mxc-sdk -> mxc_ffi path), so its dead `SandboxUserCredentials` is tracked as a
separate deliverable.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0c82ca12-a566-4257-9de7-164be299375e
…gration Collapses the review-round fixes for the IsolationSession Preview API migration into a single change. Grouped by what they fix. Lifetime and resource safety: * Do not leak the agent user when provisioning fails after the account is minted. `add_user` now returns the provisioned user alongside the manager so every early-return path can deprovision it, and the post-mint failure paths run a best-effort deprovision. * Stop passing a stack pointer to the stdio relay threads. The relay cannot be interrupted while blocked in `ReadFile`, so no join can be guaranteed to complete and any timed-out join returns into the same use-after-free. The parameters are heap-owned instead, which removes the window rather than narrowing it. Policy correctness: * Emit `ui` only when the caller supplied one, so an omitted `ui` is not silently materialised into a lockdown policy the backend would then refuse. Covered by regression tests. Test-gate correctness -- several gates were passing without testing anything: * Probe the backend by asking the binary under test, and treat a missing or non-boolean probe value as "unavailable" rather than truthy. The previous gate keyed on a hard-coded DLL path and WinRT class registry key, which stopped tracking what the code activates once the backend moved to the Preview API. * Make the state-aware config exhaustiveness guard actually fire. It asserted through `x as never`, which is always a legal assertion, so the guard could never fail; the switch subject must be a bare reference for narrowing to apply. Likewise distribute the all-optional-config check over the backend union, since a single all-optional member otherwise satisfies it for every backend. * Skip the policy-validation suite on a build without the isolation_session feature instead of failing it, while still running it on hosts that merely lack IsolationSession runtime support -- those refusals are raised before any OS call, and that is the coverage the suite exists for. Its gate is computed above every `describe`, because the suite runs with `--test-force-exit` and a `describe` registering after a top-level `await` is dropped silently, with exit 0. Also repairs the SDK integration build, makes required state-aware config explicit in the TypeScript surface, and corrects the state-aware design docs and worked examples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c82ca12-a566-4257-9de7-164be299375e
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Migrates the IsolationSession backend + SDK surface to the Windows.AI.IsolationSession.Preview in-proc API, aligning policy semantics and wire contracts with what the Preview API can actually enforce.
Changes:
- Replaces IsolationSession availability checks with
wxc-exec --probe(probes.isolationSessionAvailable) and updates scripts/docs accordingly. - Updates wire/schema + SDK types for new IsolationSession lifecycle shape (network acknowledgment required at provision; removed sizing/user config; new
appId; new structured error fields). - Refactors backend implementation (removes folder sharing/protected path filter; adjusts lifecycle + relay thread ownership; adds structured API-failure fields).
Reviewed changes
Copilot reviewed 99 out of 102 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/scripts/run_isolation_session_resize_smoke.ps1 | Switches skip gating to wxc-exec --probe and adds canonical network acknowledgment to configs. |
| tests/scripts/README.md | Documents new skip semantics for IsolationSession suites. |
| tests/configs/isolation_session_timeout.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_streaming_smoke.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_stdout_stderr_interleaved.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_stderr.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_state_aware_start_upn_mismatch.json | Removes obsolete Entra/start mismatch config. |
| tests/configs/isolation_session_state_aware_start_medium.json | Removes obsolete sizing-profile config. |
| tests/configs/isolation_session_state_aware_start_local_with_user.json | Removes obsolete Entra user start config. |
| tests/configs/isolation_session_state_aware_start_entra_missing_user.json | Removes obsolete Entra/start missing user config. |
| tests/configs/isolation_session_state_aware_provision_with_filter.json | Removes obsolete filesystem filter scenario. |
| tests/configs/isolation_session_state_aware_provision_user_malformed_upn.json | Replaces Entra user test with appId validation + required network ack. |
| tests/configs/isolation_session_state_aware_provision_user_empty_wamtoken.json | Replaces Entra token test with appId and required network ack. |
| tests/configs/isolation_session_state_aware_provision_rejected_ui.json | Adds state-aware UI-policy refusal config. |
| tests/configs/isolation_session_state_aware_provision_rejected_network.json | Adds state-aware network-policy refusal config. |
| tests/configs/isolation_session_state_aware_provision_appid_too_long.json | Adds state-aware appId length validation config. |
| tests/configs/isolation_session_state_aware_provision_appid_control.json | Adds state-aware appId control-character validation config. |
| tests/configs/isolation_session_state_aware_provision.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_state_aware_exec_write_shared.json | Removes obsolete host folder sharing exec scenario. |
| tests/configs/isolation_session_state_aware_exec_write_readonly_denied.json | Removes obsolete host folder sharing exec scenario. |
| tests/configs/isolation_session_state_aware_exec_read_shared.json | Removes obsolete host folder sharing exec scenario. |
| tests/configs/isolation_session_state_aware_exec_read_restricted.json | Removes obsolete host folder sharing exec scenario. |
| tests/configs/isolation_session_state_aware_exec_read_readonly.json | Removes obsolete host folder sharing exec scenario. |
| tests/configs/isolation_session_powershell_interactive.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_one_shot_user_rejected.json | Removes obsolete one-shot Entra user rejection config. |
| tests/configs/isolation_session_one_shot_ui_rejected.json | Adds one-shot UI-policy refusal config. |
| tests/configs/isolation_session_one_shot_stray_config_ignored.json | Adds one-shot “stray backend config ignored” config. |
| tests/configs/isolation_session_one_shot_network_rejected_no_local.json | Adds one-shot network refusal config (missing allowLocalNetwork). |
| tests/configs/isolation_session_one_shot_network_rejected_hosts.json | Adds one-shot network refusal config (host rules). |
| tests/configs/isolation_session_one_shot_network_rejected.json | Adds one-shot network refusal config (block). |
| tests/configs/isolation_session_one_shot_lifecycle_rejected.json | Adds one-shot lifecycle refusal config. |
| tests/configs/isolation_session_hello_medium.json | Updates “hello” config to new policy shape and removes sizing/env inline formatting. |
| tests/configs/isolation_session_hello.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_filtered.json | Removes obsolete filesystem filtering config. |
| tests/configs/isolation_session_filesystem.json | Removes obsolete filesystem sharing config. |
| tests/configs/isolation_session_exit42.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_concurrent_D.json | Adds required unrestricted-network acknowledgment. |
| tests/configs/isolation_session_concurrent_C.json | Adds required unrestricted-network acknowledgment and removes filesystem mapping. |
| tests/configs/isolation_session_concurrent_B.json | Adds required unrestricted-network acknowledgment and removes filesystem mapping. |
| tests/configs/isolation_session_concurrent_A.json | Adds required unrestricted-network acknowledgment and removes filesystem mapping. |
| src/testing/wxc_e2e_tests/tests/e2e_isolation_session_policy.rs | Adds E2E policy-refusal tests that run pre-OS-call validation paths. |
| src/core/wxc_common/src/wire.rs | Removes sizing/user config; introduces provision-only appId wire type. |
| src/core/wxc_common/src/state_aware_request.rs | Removes tests tied to Entra user secret-redaction paths. |
| src/core/wxc_common/src/state_aware_dispatch.rs | Adds dry-run tests ensuring validation runs but phase body is skipped. |
| src/core/wxc_common/src/mxc_error.rs | Adds structured API-failure fields (operation, nativeCode, remediation) and flattens into wire envelope. |
| src/core/wxc_common/src/models.rs | Removes IsolationSession domain config; adds network_specified/ui_specified presence flags; adds provision app_id. |
| src/core/wxc_common/src/config_parser.rs | Captures network/ui presence flags and removes one-shot IsolationSession domain-config mapping. |
| src/core/wxc_common/src/config_deserialize.rs | Updates secret-marker docs/tests and keeps redaction behavior. |
| src/core/wxc/src/main.rs | Overrides probe result with IsolationSession activation probe when feature compiled. |
| src/core/mxc_engine/src/policy.rs | Emits ui only when caller supplied it; adds tests for presence-based behavior. |
| src/core/mxc_engine/src/platform.rs | Adds engine-level isolation_session_available() probe. |
| src/core/mxc_engine/src/lib.rs | Re-exports isolation_session_available under feature/OS gate. |
| src/backends/isolation_session/common/src/protected_paths_filter.rs | Deletes mitigation file as folder sharing is removed. |
| src/backends/isolation_session/common/src/process_options.rs | Refactors error mapping to include operation identifiers. |
| src/backends/isolation_session/common/src/pipe_relay.rs | Ensures relay thread owns/free params via heap allocation to avoid lifetime hazards. |
| src/backends/isolation_session/common/src/one_shot.rs | Removes sizing/user config; provisions via OS-assigned user; rejects unsupported lifecycle flags. |
| src/backends/isolation_session/common/src/lib.rs | Updates backend docs/module layout; removes folder sharing module; exposes service-availability probe. |
| src/backends/isolation_session/common/src/folder_sharing.rs | Deletes folder-sharing helpers (API no longer available). |
| src/backends/isolation_session/common/src/console_relay.rs | Ensures console relay thread owns/free params to keep COM refs alive safely. |
| src/backends/isolation_session/common/Cargo.toml | Adds serde_json/base64 deps required for new sandboxId/appId handling. |
| src/backends/isolation_session/bindings/src/lib.rs | Updates bindings docstrings to Preview API naming. |
| src/backends/isolation_session/bindings/build.rs | Fixes paths to provenance + Cargo.lock for version checks. |
| src/backends/isolation_session/bindings/Cargo.toml | Narrows windows dependency features for bindings. |
| src/backends/appcontainer/common/src/probe.rs | Adds isolationSessionAvailable probe field (default false) for SDK gating. |
| sdk/node/tests/unit/wire-conformance-state-aware.test.ts | Updates state-aware wire conformance assertions for provision-only phase type. |
| sdk/node/tests/unit/state-aware.test.ts | Updates state-aware behavior tests (appId, structured errors, required network ack). |
| sdk/node/tests/unit/state-aware-types.test.ts | Updates SDK types tests for required network ack and appId; removes Entra user type. |
| sdk/node/tests/unit/platform.test.ts | Replaces build-number gate tests with probe-based IsolationSession availability gate. |
| sdk/node/tests/unit/errors.test.ts | Adds tests for structured MxcError fields and envelope mapping helper. |
| sdk/node/tests/integration/test-helpers.ts | Ensures runtime probes pass the minimum valid config (iso requires network ack); adds feature-only probe helper. |
| sdk/node/src/state-aware.ts | Makes provision config conditionally required; parses structured error envelopes. |
| sdk/node/src/state-aware-types.ts | Removes Entra user config class; adds required network ack + appId + structured conditional types. |
| sdk/node/src/state-aware-helper.ts | Parses structured error fields into MxcError. |
| sdk/node/src/sandbox.ts | Uses structured envelope mapping helper for non-state-aware errors too. |
| sdk/node/src/platform.ts | Removes pinned Windows build gate; adds probe-based method availability. |
| sdk/node/src/index.ts | Exports MxcErrorFields; stops exporting removed IsolationSessionUserConfig. |
| sdk/node/src/generated/wire.ts | Regenerates wire types for new IsolationSession config shape. |
| sdk/node/src/errors.ts | Adds structured MxcError fields + mxcErrorFromEnvelope helper. |
| sdk/node/README.md | Updates docs links and state-aware example (required network ack + structured failures). |
| schemas/dev/mxc-config.schema.0.8.0-dev.json | Regenerates schema removing old iso config and adding provision-only appId. |
| external/windows-sdk/isolation-session/GENERATION_INFO.toml | Updates generation date for new bindings snapshot. |
| docs/windows-sandbox/windows-sandbox.md | Removes outdated reference to unsupported Entra bundle. |
| docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md | Updates error model and IsolationSession phase config examples. |
| docs/schema.md | Adds UI policy documentation and clarifies backend rejection/ignore semantics. |
| docs/schema-codegen.md | Updates schema/codegen description to match new wire model. |
| docs/sandbox-policy/v2/networking.md | Updates IsolationSession networking policy semantics to required allow-ack model. |
| docs/isolation-session/state-aware-typescript-initial-plan.md | Updates TS plan doc to new spec and removes Entra user coverage. |
| docs/isolation-session/state-aware-rust-initial-plan.md | Removes obsolete initial plan doc (replaced by spec). |
| .github/copilot-instructions.md | Updates backend matrix documentation to new IsolationSession behavior + API. |
Suppressed comments (2)
src/backends/isolation_session/common/Cargo.toml:1
serde_jsonis declared in both[dependencies]and[dev-dependencies]with the same spec. This is redundant and can be removed from[dev-dependencies]unless you intend different features/versions for tests.
src/core/wxc_common/src/config_deserialize.rs:1- Typo in comment:
pathesshould bepaths.
3 tasks
Huzaifa Danish (huzaifa-d)
pushed a commit
that referenced
this pull request
Aug 7, 2026
Address @bbonaby's review notes on #717: - Define what 'Stock Windows' means (clean install, default optional features -> appcontainer-dacl floor) and link docs/process-container/os-version-support.md near the examples table. And reflect @adpa-ms's change in #761: - isolation_session availability is now detected by whether the Windows.AI.IsolationSession.Preview IsoSessionOps API class is registered on the OS (activation-factory resolves), not a build-number gate (26300.8553). Update the probe-gap table row and the follow-up work item accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Huzaifa Danish (huzaifa-d)
pushed a commit
that referenced
this pull request
Aug 7, 2026
Add isolation_session to the host-availability surfaces using @adpa-ms's registration-based approach from #761: availability is whether the in-proc Windows.AI.IsolationSession IsoSessionOps WinRT class is registered on the OS (its activation factory resolves), not a build-number gate. - New isolation_session_common::availability::is_isolation_session_available(): attempts IsoSessionOps activation (CoInitialize MTA, balanced), OnceLock-cached, elevation-free. Pure available_from() split from the COM probe for unit testing (CLASS_E_CLASSNOTAVAILABLE / REGDB_E_CLASSNOTREG and any other activation failure map to unavailable). - Wire it into both Windows host-capability surfaces, gated behind the engine's isolation_session feature: available_backends() (probe) and platform_support(). This closes the previously-documented Rust/TS parity gap where TS reported isolation_session but Rust did not. - Add isolation_session to the probe's EMITTABLE_BACKENDS drift guard and loosen the mxc-sdk Windows platform_support test to allow it. Validated with the feature on and off: fmt clean, clippy -D warnings clean (default, isolation_session, and wslc+tier2_bfs+isolation_session), and tests green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
8 tasks
Huzaifa Danish (huzaifa-d)
added a commit
that referenced
this pull request
Aug 7, 2026
* Add backend support probe API design & discussion doc Design-only doc for a read-only Rust available_backends() host-capability probe: API shape, isolation-tier ceiling model, current per-backend detection methods and their risks, the remaining probe gap, testing, and follow-up work. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Added Rust vs TS * Apply batched suggestions from Copilot's code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * docs: require side-effect-free transport in backend probe API plan The §4.2 step 2 previously said to "extend that JSON to carry the backend list" via `wxc-exec --probe`. This violates the proposed read-only contract: `wxc/src/main.rs:750-772` runs `recover_orphaned_state()` *before* handling `--probe`, which can restore/prune host DACL state. Update the plan to: - Require a side-effect-free transport (e.g. a new `--available-backends` mode handled before DACL recovery, or `mxc_ffi`) instead of extending `--probe` unchanged. - Explicitly note the constraint in follow-up work item 5. * docs(probe): define Stock Windows + update isolation_session detection Address @bbonaby's review notes on #717: - Define what 'Stock Windows' means (clean install, default optional features -> appcontainer-dacl floor) and link docs/process-container/os-version-support.md near the examples table. And reflect @adpa-ms's change in #761: - isolation_session availability is now detected by whether the Windows.AI.IsolationSession.Preview IsoSessionOps API class is registered on the OS (activation-factory resolves), not a build-number gate (26300.8553). Update the probe-gap table row and the follow-up work item accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Huzaifa Danish <modanish@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Huzaifa Danish (huzaifa-d)
added a commit
that referenced
this pull request
Aug 7, 2026
#725) * feat(engine): port TS host detectors (lxc, windows_sandbox) to Rust Port the two TypeScript-only host-availability detectors into Rust and fold their results into mxc_engine::platform::platform_support(), broadening its contract from 'backends mxc-sdk can launch' to 'host-available backends' (Phase 1 of the backend-support-probe plan, PR #717). - Add lxc_common::availability::is_lxc_available() (shallow 'lxc-ls --version' probe). - Add windows_sandbox_lifecycle::availability::is_windows_sandbox_available() (DISM State:Enabled with a 10s timeout, WindowsSandbox.exe fallback when DISM can't run; invokes the absolute System32\dism.exe path). - platform_support(): Linux arm reports lxc and/or bubblewrap; Windows arm adds windows_sandbox alongside processcontainer. Update doc comments to host-capability wording. - Add drift-guard tests tying reported literals to Containment wire names and asserting the live platform_support() output only contains real wire names. - Loosen the locked mxc-sdk platform_support tests for the broadened contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * feat(engine): add available_backends() host-capability probe API Add mxc_engine::probe with the read-only available_backends() API from the backend-support-probe plan (PR #717, Phase 2). Reports only the containment backends the current host can run, each with its effective isolation tier when it has a tier ladder. - AvailableBackend { backend, tier: Option<String> } serializes to camelCase JSON with tier omitted (never null) when the backend has no tier ladder. - available_backends() has per-platform arms reusing the landed detectors: macOS -> seatbelt; Linux -> bubblewrap/lxc; Windows -> processcontainer (with effective tier) + windows_sandbox + wslc (feature-gated). Empty Vec is a normal result, not an error. - select_tier() is a pure precedence fn (base-container -> appcontainer-bfs -> appcontainer-dacl floor), unit-testable without a real host or the tier2_bfs feature. - Re-exported from mxc_engine and the public mxc-sdk. - 8 unit tests: serde shape (tier omitted vs present), host + unconditional wire-name drift guards, canonical-tier drift guard, Windows processcontainer-always-with-tier, tier precedence, non-Windows processcontainer absence. Stacked on the host-detector port (PR #725); the standalone detectors it reuses land there. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(windows_sandbox): address Copilot review on host detectors - Resolve dism.exe / WindowsSandbox.exe via GetSystemDirectoryW instead of the attacker-controllable %SystemRoot% env var (UAC inherits an unelevated parent's environment, so an env-derived path let a standard user point the probe at a planted binary). Mirrors the src/host/plm/src/wpr_path.rs pattern; falls back to the C:\\Windows\\System32 literal only on an outright Win32 failure. - Pass DISM's /English global option so the parsed 'State : Enabled' tokens are not localized on non-English Windows (previously an enabled Sandbox could be reported unavailable there, since a successful DISM run also skips the exe fallback). - Soften platform_support() docs: available_methods is the currently detected subset, not an exhaustive capability list — backends without a Rust detector yet (notably isolation_session) are omitted even when the host could run them, so absence is not proof a backend cannot run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: trim comments; drop DISM from windows_sandbox probe Trim the module-level narratives and over-explained item docs across the host detectors and probe to concise, necessary comments; remove duplicated phrasing (e.g. 'universal floor') within and across files. Also drop the DISM query from the Windows Sandbox probe: dism /online requires elevation and this probe only ever runs unelevated (wxc-exec does not self-elevate), so DISM always failed through to the WindowsSandbox.exe existence check anyway. Detection is now exe-only, which also removes the subprocess-launch attack surface (only a .exists() remains). A short comment records why DISM is skipped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * refactor(appcontainer): centralize isolation-tier name strings Address @bbonaby's review note (#717): the tier-name strings were hand-written across the appcontainer fallback code. Make IsolationTier the single source of truth in both directions. - Add IsolationTier::ALL (canonical tier set, strongest-first) and derive a FromStr impl from as_str() via ALL, so the two directions cannot drift and adding a tier is a one-line change to ALL + as_str. - Remove the ad-hoc test-only parse_force_tier(); the production MXC_FORCE_TIER seam now parses via FromStr. - Add typed ForceTierGuard::set_tier(IsolationTier) and migrate all 18 valid force-tier call-sites off raw string literals (the one negative test intentionally keeps a raw invalid value). - Add a round-trip test asserting every ALL tier survives as_str -> FromStr. The available_backends() probe (already merged here) consumes as_str() with its own drift guard; the CI coverage gate for new tiers/backends is tracked in the follow-up issue. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * feat(engine): add isolation_session host detection (registration-based) Add isolation_session to the host-availability surfaces using @adpa-ms's registration-based approach from #761: availability is whether the in-proc Windows.AI.IsolationSession IsoSessionOps WinRT class is registered on the OS (its activation factory resolves), not a build-number gate. - New isolation_session_common::availability::is_isolation_session_available(): attempts IsoSessionOps activation (CoInitialize MTA, balanced), OnceLock-cached, elevation-free. Pure available_from() split from the COM probe for unit testing (CLASS_E_CLASSNOTAVAILABLE / REGDB_E_CLASSNOTREG and any other activation failure map to unavailable). - Wire it into both Windows host-capability surfaces, gated behind the engine's isolation_session feature: available_backends() (probe) and platform_support(). This closes the previously-documented Rust/TS parity gap where TS reported isolation_session but Rust did not. - Add isolation_session to the probe's EMITTABLE_BACKENDS drift guard and loosen the mxc-sdk Windows platform_support test to allow it. Validated with the feature on and off: fmt clean, clippy -D warnings clean (default, isolation_session, and wslc+tier2_bfs+isolation_session), and tests green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * docs: trim comments in the windows_sandbox + isolation_session probes Keep only comments that explain non-obvious code (why exe-not-DISM, the GetSystemDirectoryW env-spoof rationale, the Win32 buffer-grow retry, and the COM init/uninit balancing); drop the rest. No behavior change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: restore platform_support module rationale Restore the 'why this exists' note (in-process host discovery, decoupled from the TypeScript SDK; lives in the engine so the SDK and executor binaries share one impl) that an earlier comment-trim over-aggressively removed. Kept concise. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: restore pre-existing platform.rs comments simplified by this PR Per review: only prune comments in files/comments this PR introduces. Restore the pre-existing comments an earlier trim pass simplified in platform.rs: - module doc (full 'stop depending on the TypeScript SDK' + 'lives alongside dispatch.rs' rationale), - platform_support() fn doc (the wxc-exec --probe and wslc/feature specifics; dropped only the now-inaccurate 'restricted to backends mxc-sdk can run' clause the broadened contract invalidated, pointing to the field doc instead), - the Linux-arm bwrap / MIN_BWRAP_VERSION note (kept, plus the new lxc line). The two sdk_helpers.rs comments were left corrected rather than reverted: this PR's behavior change made them factually wrong (LXC is now reported; WSLC is no longer the only other Windows backend). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * refactor(wxc_common): extract shared system_directory() helper Addresses jsidewhite's review on #725: GetSystemDirectoryW + grow-retry was duplicated in windows_sandbox availability.rs and plm/wpr_path.rs. Consolidate into wxc_common::system_dir and route both call-sites through it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(engine): address review on host detectors (wslc probe, tier de-drift, COM guard) - probe.rs: report wslc via wslc_common::is_available() (WSL2 + runtime check) instead of WslcSdk::load(), matching platform_support() and the runner preflight (SohamDas). - fallback_detector.rs: generate IsolationTier's enum, ALL, as_str, and FromStr from one macro list so the mapping is exhaustive both ways and cannot drift (SohamDas). - isolation_session/availability.rs: wrap CoInitializeEx/CoUninitialize in an RAII ComApartment guard so a panic in IsoSessionOps::new() still balances COM init (SohamDas). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(windows_sandbox): route runner preflight through the trusted availability probe check_sandbox_available() used an env-spoofable %SystemRoot% path while capability reporting used GetSystemDirectoryW. Delegate the preflight to is_windows_sandbox_available() so reporting and execution agree and the runner is no longer environment-spoofable (Copilot review). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(windows_sandbox): launch WindowsSandbox.exe from the trusted system path The daemon launched the sandbox binary by bare name, relying on the executable search order (app dir/CWD) — an attacker-planted binary could run in its place, outside the sandbox. Resolve it under the trusted system_directory() (GetSystemDirectoryW), matching the detection path hardened elsewhere in this PR. Surfaced by adversarial review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(engine): keep platform_support() limited to SDK-launchable backends available_backends() (introduced by this PR) is the host-capability probe; it already reports the broad set (lxc, windows_sandbox, isolation_session, wslc). platform_support() must stay limited to the backends mxc_sdk::spawn_sandbox can actually launch (dispatch.rs: seatbelt, bubblewrap, processcontainer, wslc), or a caller picking an advertised method gets a guaranteed UnsupportedContainment error. This PR had wrongly expanded platform_support() to add lxc (Linux) and windows_sandbox + isolation_session (Windows); revert those arms and the field doc to the launchable-only contract (per #717 design 7.1). The new drift tests are kept. Addresses SohamDas2021 / Copilot review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(lxc): bound and cache the lxc-ls availability probe is_lxc_available() ran `lxc-ls --version` with a blocking status() and no cache, so a hung lxc-ls would block discovery indefinitely and every call re-spawned it (unlike the isolation_session and windows_sandbox probes). Cache the result in a OnceLock and wait with a bounded deadline, killing and reaping the child on timeout. Addresses SohamDas2021 review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(engine): derive probe backend names from the enum; gate BFS tier on bfscfg.exe - #7 (SohamDas2021): available_backends() pushed hardcoded string literals, and the drift tests only checked that a separate hardcoded list was valid wire names — not that the pushed literals matched. Derive every pushed name from ContainmentBackend::wire_name() so it can't be typo'd, and make the emittable test iterate the enum so it verifies ContainmentBackend::wire_name() agrees with the canonical wire::Containment serde names. - #9 (SohamDas2021): select_tier() reported AppContainerBfs from the tier2_bfs build flag alone, but detect() also requires bfscfg.exe on disk for a policy-carrying request (else it falls to DACL). Add a bfscfg_available() probe and require both the feature and a resolvable bfscfg.exe before naming BFS, so the reported tier ceiling matches what a real request achieves. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs(mxc-sdk): add host-backend discovery usage guidance + before/after Documents platform_support() vs available_backends() (which to use when), a usage example, and the tier-ceiling caveat, plus the before/after framing. Addresses jsidewhite review on #725. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(lxc): drop unread exit code from LxcLsOutcome::ExitedFailure The decision only distinguishes success from every other outcome, so the Option<i32> exit code was never read (dead code under -D warnings). Make ExitedFailure a unit variant. Addresses Copilot review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(engine): report BFS tier ceiling from tier2_bfs alone, not bfscfg.exe Reverts the bfscfg.exe gating added earlier in this PR. The probe's tier is a CEILING (strongest reachable for some request), not a per-request value: detect() returns AppContainerBfs for a no-filesystem-policy request without bfscfg.exe, so the ceiling on any tier2_bfs build is BFS regardless of bfscfg. Gating the ceiling on bfscfg under-reported it to appcontainer-dacl on a tier2_bfs host lacking bfscfg. bfscfg only decides whether a policy-carrying request stays at BFS or drops to DACL, which is a request-time dispatch concern. Addresses Copilot review; supersedes the earlier response to the bfscfg comment. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test(mxc-sdk): fence platform_support() to the SDK-launchable set The platform_support consistency tests were loosened earlier in the PR when it temporarily advertised lxc / windows_sandbox / isolation_session; after those moved to available_backends() the tests were never re-tightened, so they still permitted the excluded backends and a regression re-advertising one would pass (the Linux loop also passed vacuously on a host with no methods). - Linux: assert available_methods == exactly ["bubblewrap"] (lxc excluded). - Windows: keep processcontainer-first, then assert every method is processcontainer | wslc, explicitly excluding windows_sandbox / isolation_session. Addresses SohamDas2021 review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Huzaifa Danish <modanish@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
Migrates the
isolation_sessionbackend and its SDK surface to theWindows.AI.IsolationSession.Previewin-proc runtime API, and brings the backend's policy behaviour in line with what that API can actually deliver.Lifecycle shape. The agent user account name is now OS-assigned rather than client-minted: provision returns it, along with the agent SID and an OS-provided
ephemeralWorkspacePath— a directory shared between the calling user and the isolated agent user. Sizing profiles (configurationId) have no equivalent in the Preview API and are removed, leavingprovisionas the only phase that carries a per-phase config.Host folder sharing is gone. The Preview API exposes no folder-sharing primitive, so
readwritePaths/readonlyPaths/deniedPathsare refused at every phase rather than partially honoured; staging files in and out of a session goes through the ephemeral workspace instead. This also retires the emergency path filter added for the subtree-ACE regression, because the call it guarded no longer exists.Three policy sections move from "silently dropped" to "explicitly refused", so the backend never appears to offer a guarantee it cannot enforce.
network— the container's network is unrestricted on both axes (outbound is open, and a process inside can listen on a localhost-reachable port), and MXC has no primitive to filter or deny it. Provision accepts only the canonical unrestricted-network acknowledgment (defaultPolicy: allow+allowLocalNetwork: true, no host rules, no proxy, default enforcement) and refuses everything else — including an absent policy, which would otherwise default to an unenforceableblock. Post-provision phases reject any supplied network policy and inherit the posture fixed at provision.ui— an isolation session isolates the host's UI from contained code, but does not deny that code UI capabilities: window creation, GDI and the session's own clipboard all work inside it. Nouiposture would be truthful, so a supplieduiis refused at every phase on both surfaces. Unlikenetworkthere is no acknowledgment form, because there is no value a caller could supply that would be honest — so there is nothing to accept. The refusal is presence-based rather than value-based:UiPolicy's defaults are full lockdown, which makes an explicitly-supplied lockdownuiindistinguishable by value from an absent one. An omitteduiis accepted and applies no restriction — note this means the schema's default-deny reading does not hold on this backend.lifecycle— the in-proc API exposes no session-lifetime knob, so one-shot refusesdestroyOnExit: falseandpreservePolicy: true(the defaultdestroyOnExit: truematches actual behaviour and is accepted), and the state-aware parser refuses the section outright.sandboxIdbecomes opaque, structured and versioned —iso:<base64url-nopad(JSON)>— carrying the OS-assigned agent user name plus an optional caller-suppliedappId(the Package Family Name for a packaged application).appIdis validated structurally and is carried, not consumed: it exists so that a future OS contract keyed on calling-application identity does not require a breaking id change. Ids minted by a newer MXC are reported as such rather than as corrupt. Pre-migration plaintextiso:<name>ids are no longer addressable — an accepted consequence of the format change, since no sandbox survives an executor upgrade.State-aware failures carry structured fields on the wire envelope —
operation,nativeCode,remediation— as siblings ofcodeandmessagerather than prose folded into a single string, so callers can branch without parsing text. The invariant is thatnativeCodeandremediationnever appear withoutoperation; a failure MXC raises before any API call carries neither.0x80070490maps tostale_idrather than a generic backend error because, on the lifecycle operations, it is raised only for an unknown agent user — every other failure mode on those paths, including a missing executable, surfaces a different HRESULT.Host support detection moves from a hardcoded Windows build-number gate in the TypeScript SDK to a runtime probe (
wxc-exec --probe→probes.isolationSessionAvailable). The old gate pinned an exact Insider build and would have reported "unsupported" on any newer one.Known gaps and deliberate exclusions
These were considered and consciously left out; they are not oversights.
userbundle ({ upn, wamToken }) is not carried here.sdk/dotnet/) is not updated for the new wire contract. It still emitsconfigurationIdand auserbundle for this backend, and its comments describe the pre-migration shape. No files undersdk/dotnet/are touched by this PR. Nothing breaks at runtime, because the C# path cannot reach an experimental backend today. Tracked separately.STILL_ACTIVE(259) exit-code ambiguity in the graceful-shutdown wait, and the fact that a bounded join on the stdio relay threads is best-effort by construction — a relay blocked in a read cannot be interrupted, so no join can be guaranteed to complete. Relatedly, the relay handle lifetime is sound for a one-shot process but deserves revisiting for a long-lived state-aware process performing manyexeccalls, where handle values are recycled aggressively. That is a follow-up, not a regression introduced here.🔗 References
No tracking issue. These commits were previously reviewed as PRs into the feature branch: #592, #683, #682, #708, #718, #746.
🔍 Validation
fmt,clippy --all-targets --all-features -D warnings, and build + test for x64 withisolation_sessionON and OFF, plus an aarch64 build. The OFF configuration is covered deliberately: a change that only compiles with the feature enabled would otherwise reachmainunnoticed.Versioning and codegen gates all pass: schema versions, schema codegen, SDK wire-types codegen, config validation, version sync, and toolchain sync.
cargo fetch --lockedresolves the whole graph through the public feed across all five CI target triples.SDK unit tests and the SDK integration suite pass. On a host with isolation-session runtime support: one-shot suite 16, state-aware lifecycle suite 62, SDK integration 12 — 90 passed, 0 failed, 0 skipped — followed by a manual interactive pass covering TTY resize, streaming, and interactive PowerShell. A name-agnostic diff of local accounts taken before and after the run showed no leaked agent accounts.
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow