From 6d3538bae89a523d022256740d5b27fb22e95718 Mon Sep 17 00:00:00 2001 From: Gudge Date: Wed, 24 Jun 2026 16:41:44 -0700 Subject: [PATCH 1/3] Phase 2B: deserialize the parser into the wire model This PR changes the config parser to deserialize directly into the typed wire model (`wxc_common::wire::MxcConfig`) instead of an intermediate set of permissive structs, so the JSON schema source of truth and the parser's trust boundary share a single definition of the wire shape and cannot drift. Details: - The parser deserializes both one-shot and state-aware requests straight into `wire::MxcConfig`, discriminating state-aware requests by the `phase` key and keeping the `experimental` block as a raw value for per-backend dispatch typing. - The wire structs are the deserialization target; the previous intermediate structs and their conversion helpers are removed (a large net reduction in `config_parser.rs`). - The boundary is strict by construction: invalid containment, network, clipboard, and configurationId values are rejected, and unknown nested fields are rejected (`deny_unknown_fields`). Deprecated spellings (`appcontainer`, `macos_sandbox`, `appContainer`) are accepted via serde aliases. - The `wire` module is always compiled as the parser target; its `JsonSchema` derive stays behind the `schema-gen` feature. - The TypeScript SDK and the in-process `mxc-sdk` crate emit only fields the wire objects define: the container id is carried at top-level `containerId` and the teardown / preserve flags at `lifecycle.destroyOnExit` / `lifecycle.preservePolicy`, so `processContainer.name`, `lxc.containerName`, `lxc.destroyOnExit`, and `filesystem.clearPolicyOnExit` are no longer sent. - Docs (`versioning.md`, `authoring-a-new-feature.md`, `schema-codegen.md`, `copilot-instructions.md`, state-aware lifecycle) describe the wire-model parser. Tests: - cargo test --workspace; cargo fmt --all -- --check; cargo clippy --workspace --all-targets -- -D warnings. - Schema codegen, config corpus (169/169), and schema-version gates. - SDK unit tests (cd sdk && npm test). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8 --- .github/copilot-instructions.md | 6 +- docs/authoring-a-new-feature.md | 181 +- docs/bwrap-support/bubblewrap-backend-plan.md | 21 +- .../nanvix-microvm/nanvix-integration-plan.md | 5 +- docs/schema-codegen.md | 13 +- .../mxc-state-aware-sandbox-api-overview.md | 10 +- .../mxc-state-aware-sandbox-api.md | 113 +- docs/versioning.md | 102 +- schemas/dev/mxc-config.schema.0.8.0-dev.json | 8 +- sdk/src/sandbox.ts | 16 +- sdk/tests/unit/sandbox.test.ts | 10 +- src/core/mxc-sdk/src/policy.rs | 4 +- src/core/wxc_common/src/config_parser.rs | 1494 ++++++++--------- src/core/wxc_common/src/lib.rs | 6 +- .../wxc_common/src/state_aware_request.rs | 36 - src/core/wxc_common/src/wire.rs | 132 +- 16 files changed, 995 insertions(+), 1162 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3e8b020fd..e75c52deb 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -109,7 +109,7 @@ The Rust workspace (`src/`) implements multiple sandboxing backends behind the ` ### Config flow -1. User provides JSON config (file or base64) → `config_parser.rs` deserializes into intermediate `Raw*` structs → validates and maps to `ExecutionRequest` (the internal execution model in `models.rs`) +1. User provides JSON config (file or base64) → `config_parser.rs` deserializes into the typed wire model (`wxc_common::wire`) → validates and maps to `ExecutionRequest` (the internal execution model in `models.rs`) 2. `ExecutionRequest` includes the containment backend selection, process config, filesystem/network policies, and optional experimental features 3. The appropriate `ScriptRunner` implementation executes the process and returns `ScriptResponse` @@ -163,7 +163,7 @@ State-aware lifecycle: New features go under the `experimental` JSON section and are only active when `--experimental` is passed. See `docs/authoring-a-new-feature.md` for the full checklist. The pattern: 1. Add the field to the Rust wire model (`src/core/wxc_common/src/wire.rs`) under the `Experimental` section, then regenerate the dev schema (`cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- schemas/dev/mxc-config.schema..json`) — do not hand-edit the generated schema -2. Add Rust structs to `models.rs` (`ExperimentalConfig`) and `config_parser.rs` (`RawExperimental`) +2. Add the matching field to the wire model's `Experimental` struct (`src/core/wxc_common/src/wire.rs`) and the domain `ExperimentalConfig` in `models.rs`, then map wire→domain in `config_parser.rs` 3. Guard execution behind `if request.experimental_enabled` in the runner 4. Never modify files in `schemas/stable/` — those are immutable release artifacts @@ -191,7 +191,7 @@ The workspace is organized into five top-level directories under `src/`: ### Config parser pattern -The parser uses two layers of structs: `Raw*` structs (matching JSON with `#[serde(rename)]`) that deserialize permissively, then map to validated domain structs in `models.rs`. This keeps serde attributes separate from the internal model. +The parser deserializes JSON directly into the typed wire model (`wxc_common::wire`), the single source of truth for the config shape (it also generates the JSON schema). `config_parser.rs` then maps the wire types to the validated domain structs in `models.rs`. The stable surface uses `deny_unknown_fields` (closed); the `experimental` block is permissive. ### TypeScript conventions diff --git a/docs/authoring-a-new-feature.md b/docs/authoring-a-new-feature.md index b002be2c7..ca31d35af 100644 --- a/docs/authoring-a-new-feature.md +++ b/docs/authoring-a-new-feature.md @@ -112,67 +112,52 @@ Adding a feature may touch these files: | File | What to change | |------|----------------| -| `schemas/dev/mxc-config.schema.0.8.0-dev.json` | Add `gpuIsolation` as a feature under `experimental` | +| `src/core/wxc_common/src/wire.rs` | Add a `gpuIsolation` field + `GpuIsolation` struct to the wire `Experimental` model (the schema is generated from this) | +| `schemas/dev/mxc-config.schema.0.8.0-dev.json` | **Generated** — regenerate with `mxc_schema_gen` after editing the wire model; do not hand-edit | | `src/core/wxc_common/src/models.rs` | Add `GpuIsolationConfig` struct, add field to `ExperimentalConfig` | -| `src/core/wxc_common/src/config_parser.rs` | Add `gpuIsolation` field to `RawExperimental` | +| `src/core/wxc_common/src/config_parser.rs` | Map the new wire field to the domain struct in `convert_wire_config` | | Runner (`appcontainer.rs` or `lxc_runner.rs`) | Feature logic, guarded behind `experimental_enabled` | | `tests/configs/` | Test config exercising your feature | -## Step 1: Update the schema +## Step 1: Add the field to the wire model (the schema source of truth) -In `schemas/dev/mxc-config.schema.0.8.0-dev.json`, the `experimental` section already -exists with `compartments` as a feature. Add `gpuIsolation` as a new -feature with its own typed schema: +The JSON schema is **generated** from the Rust wire model +(`src/core/wxc_common/src/wire.rs`); you never hand-edit +`schemas/dev/mxc-config.schema.0.8.0-dev.json`. Add your feature as a typed field +on the wire `Experimental` struct (which is intentionally permissive — no +`deny_unknown_fields` — so in-flux experimental shapes stay forward-compatible): -```json -"experimental": { - "type": "object", - "description": "Experimental features. Only active when --experimental is passed.", - "properties": { - "compartments": { - "type": "object", - "description": "Network compartment isolation (experimental).", - "properties": { - "namespace": { - "type": "string", - "description": "Compartment namespace identifier." - }, - "isolationLevel": { - "type": "integer", - "minimum": 1, - "description": "Isolation level (1 = shared network, 2 = separate stack, 3 = full isolation)." - } - } - }, - "gpuIsolation": { - "type": "object", - "description": "GPU device isolation (experimental).", - "properties": { - "deviceIndex": { - "type": "integer", - "minimum": 0, - "description": "GPU device index to assign to the container." - }, - "memoryLimitMb": { - "type": "integer", - "minimum": 0, - "description": "GPU memory limit in megabytes. 0 = no limit." - }, - "allowCuda": { - "type": "boolean", - "default": false, - "description": "Allow CUDA runtime access inside the container." - } - }, - "required": ["deviceIndex", "memoryLimitMb"] - } - } +```rust +// in wire.rs +pub struct Experimental { + pub compartments: Option, + pub gpu_isolation: Option, // ← add this + // ... +} + +/// GPU device isolation (experimental). +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +pub struct GpuIsolation { + /// GPU device index to assign to the container. + pub device_index: Option, + /// GPU memory limit in megabytes. 0 = no limit. + pub memory_limit_mb: Option, + /// Allow CUDA runtime access inside the container. + pub allow_cuda: Option, } ``` -Each experimental feature is its own typed property under `experimental` — -the same pattern as stable features (`filesystem`, `network`) under the -top-level config. This gives editors full autocomplete and validation. +The `///` doc comments become schema `description`s and `#[schemars(...)]` +attributes become constraints. Then regenerate the committed schema: + +``` +cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- schemas/dev/mxc-config.schema.0.8.0-dev.json +``` + +The `check-schema-codegen.js` CI gate fails if the committed schema drifts from +the wire model, so the regenerate step is mandatory. ## Step 2: Add the model struct @@ -198,74 +183,39 @@ pub struct ExperimentalConfig { } ``` -## Step 3: Parse the experimental section - -In `src/core/wxc_common/src/config_parser.rs`, the `RawExperimental` struct already -exists with `compartments`. Add `gpu_isolation`: - -```rust -#[derive(Deserialize, Default)] -struct RawExperimental { - compartments: Option, // existing - #[serde(rename = "gpuIsolation")] - gpu_isolation: Option, // ← add this -} - -#[derive(Deserialize)] -struct RawGpuIsolation { - #[serde(rename = "deviceIndex")] - device_index: u32, - #[serde(rename = "memoryLimitMb")] - memory_limit_mb: u32, - #[serde(rename = "allowCuda")] - allow_cuda: Option, -} -``` +## Step 3: Map the wire field to the domain model -In `convert_raw_config()`, map it directly — no name matching needed. Each -feature should own its parsing via a constructor: +The parser deserializes JSON directly into the wire model (`wire::MxcConfig`), +so there is no separate `Raw*` struct to add — your `wire::GpuIsolation` from +Step 1 is the parse target. In `config_parser.rs`, map it to the domain struct +inside `convert_wire_config` where the `experimental` block is converted: ```rust -let mut experimental = ExperimentalConfig::default(); - -if let Some(raw_exp) = raw.experimental { - if let Some(c) = raw_exp.compartments { - experimental.compartments = Some(CompartmentsConfig::from_raw(c)?); - } - if let Some(g) = raw_exp.gpu_isolation { - experimental.gpu_isolation = Some(GpuIsolationConfig::from_raw(g)?); +let experimental = if let Some(raw_exp) = cfg.experimental { + // ... existing feature mappings ... + let gpu_isolation = raw_exp.gpu_isolation.map(|g| GpuIsolationConfig { + device_index: g.device_index.unwrap_or(0), + memory_limit_mb: g.memory_limit_mb.unwrap_or(0), + allow_cuda: g.allow_cuda.unwrap_or(false), + }); + ExperimentalConfig { + // ... existing fields ... + gpu_isolation, } -} +} else { + ExperimentalConfig::default() +}; ``` -Each feature implements its own `from_raw()` constructor to keep -`convert_raw_config()` clean: - -```rust -impl GpuIsolationConfig { - fn from_raw(raw: RawGpuIsolation) -> Result { - Ok(Self { - device_index: raw.device_index, - memory_limit_mb: raw.memory_limit_mb, - allow_cuda: raw.allow_cuda.unwrap_or(false), - }) - } -} -``` +Prefer destructuring the wire struct (`let wire::GpuIsolation { device_index, .. }`) +in any standalone mapping helper so that adding a wire field without mapping it +becomes a compile error rather than a silent runtime drop. Add tests to verify: -- `gpuIsolation` config parses correctly +- `gpuIsolation` config parses correctly and maps to `ExecutionRequest.experimental` - Missing optional fields use defaults -- Unknown fields under `experimental` are ignored (forward compatibility) - -Also ensure that `convert_raw_config()` populates `ExecutionRequest.experimental`: - -```rust -Ok(ExecutionRequest { - // ... existing fields ... - experimental, // ← include the parsed experimental config -}) -``` +- Unknown fields under `experimental` are tolerated (forward compatibility — the + experimental surface is intentionally permissive) ## Step 4: Implement the feature in the runner @@ -374,10 +324,13 @@ The SDK passes `--experimental` to the underlying binary when this is set. When your experimental feature is ready to ship: -1. Move the property from `experimental` to a top-level property in the schema - (e.g., `experimental.gpuIsolation` → `gpuIsolation`) +1. Move the field from the wire `Experimental` struct to the top-level + `MxcConfig` (e.g., `experimental.gpuIsolation` → top-level `gpuIsolation`), + then regenerate the schema with `mxc_schema_gen` 2. Move the struct from `ExperimentalConfig` to `ExecutionRequest` -3. Move the field from `RawExperimental` to `RawConfig` +3. Map the now-top-level wire field in `convert_wire_config` (and add + `deny_unknown_fields` to the wire struct so the promoted, stable surface is + closed) 4. Remove the `if request.experimental_enabled` guard 5. Bump the minor version 6. Add a parser error for configs still referencing the feature under diff --git a/docs/bwrap-support/bubblewrap-backend-plan.md b/docs/bwrap-support/bubblewrap-backend-plan.md index a366ef601..f17b06aed 100644 --- a/docs/bwrap-support/bubblewrap-backend-plan.md +++ b/docs/bwrap-support/bubblewrap-backend-plan.md @@ -75,11 +75,19 @@ A backend-specific config can be added later under `ExperimentalConfig` if neede ### 3. Config Parser Changes -**File:** `src/core/wxc_common/src/config_parser.rs` - -- Add `RawBubblewrap` struct (placeholder — empty for now, reserved for future backend-specific fields) -- Add `bubblewrap` field to `RawExperimental` (optional, for future use) -- Add `"bubblewrap"` arm to containment match in `convert_raw_config_inner()` +> **Note (superseded):** This plan predates the wire-model parser rewire. The +> parser no longer uses `Raw*` structs or `convert_raw_config_inner`; it +> deserializes into `wire::MxcConfig` (`src/core/wxc_common/src/wire.rs`) and maps +> to the domain model in `convert_wire_config`. Translate the steps below to the +> wire model — see [authoring-a-new-feature.md](../authoring-a-new-feature.md). + +**File:** `src/core/wxc_common/src/wire.rs` and `config_parser.rs` + +- Add a `Bubblewrap` variant to the wire `Containment` enum (or rely on the + abstract `process` intent resolving to `Bubblewrap` on Linux) +- Add any backend-specific fields to the wire model (under `experimental` while + experimental), then regenerate the schema with `mxc_schema_gen` +- Map the new `containment` value in `map_wire_containment` - Optionally: make `"process"` resolve to `Bubblewrap` on Linux when LXC is unavailable (or add a `"process"` → bwrap fallback chain) @@ -337,7 +345,8 @@ policy gap is a design decision, not an implementation challenge. - `src/core/lxc/Cargo.toml` — add `bwrap_common` dependency - `src/core/lxc/src/main.rs` — add dispatch arm for `ContainmentBackend::Bubblewrap` - `src/core/wxc_common/src/models.rs` — add `Bubblewrap` variant, `BubblewrapConfig` struct, wire into `ExperimentalConfig` and `ExecutionRequest` -- `src/core/wxc_common/src/config_parser.rs` — add `RawBubblewrap`, parsing, containment match arm +- `src/core/wxc_common/src/wire.rs` — add the `Bubblewrap` containment variant (and any backend fields), then regenerate the schema +- `src/core/wxc_common/src/config_parser.rs` — map the new containment value in `map_wire_containment` ### Schema (modify) - `schemas/dev/mxc-config.schema.0.6.0-dev.json` — add `"bubblewrap"` to enum, add config block diff --git a/docs/nanvix-microvm/nanvix-integration-plan.md b/docs/nanvix-microvm/nanvix-integration-plan.md index d081502f6..cb8b1f144 100644 --- a/docs/nanvix-microvm/nanvix-integration-plan.md +++ b/docs/nanvix-microvm/nanvix-integration-plan.md @@ -115,7 +115,8 @@ mxc/src/ │ └── src/ │ ├── lib.rs # Add: pub mod nanvix_runner (1 line) │ ├── models.rs # Add: NanVixConfig struct, ContainmentBackend::MicroVm -│ ├── config_parser.rs # Add: RawNanVix struct, "microvm" parsing +│ ├── wire.rs # Add: MicroVm containment variant (schema source); regenerate schema +│ ├── config_parser.rs # Add: map_wire_containment "microvm" arm │ ├── error.rs # Add: WxcError::NanVix variant │ ├── nanvix_runner.rs # NEW — NanVixScriptRunner implementation │ ├── appcontainer.rs # UNCHANGED @@ -239,7 +240,7 @@ Setup scripts (PowerShell & Bash) will download matching pre-release binaries an **What changed:** - `models.rs` — Added `MicroVm` variant to `ContainmentBackend`, added `NanVixConfig` struct, added `nanvix_config` field to `ExecutionRequest` -- `config_parser.rs` — Added `RawNanVix` serde struct, `"microvm"` containment parsing, NanVix config section parsing +- `config_parser.rs` — Added `"microvm"` containment parsing and NanVix config section parsing (originally via `Raw*` structs; the parser has since been rewired onto the `wire::MxcConfig` model — new work maps the wire types in `convert_wire_config`) - `error.rs` — Added `WxcError::NanVix(String)` variant - `nanvix_runner.rs` — **NEW** — `NanVixScriptRunner` implementing `ScriptRunner` trait - `lib.rs` — Added `pub mod nanvix_runner` diff --git a/docs/schema-codegen.md b/docs/schema-codegen.md index 738dcea68..a5ec9f0b6 100644 --- a/docs/schema-codegen.md +++ b/docs/schema-codegen.md @@ -105,10 +105,9 @@ CI run, and the corpus gate pins the accept-side behavior. ## Roadmap - The wire model generates the committed dev schema, guarded by the codegen and - corpus CI gates. The parser still deserializes via a separate set of - permissive `Raw*` structs. -- Next: rewire the parser to deserialize directly into the wire model and delete - the `Raw*` structs, so the schema source and the trust boundary share one - definition of the wire shape and cannot drift. -- After that: generate the SDK TypeScript types from the same schema and retire - the hand-maintained `*-strict.json` stable view. + corpus CI gates. +- The parser deserializes directly into the wire model and the `Raw*` structs + are gone, so the schema source and the trust boundary share one definition of + the wire shape and cannot drift. +- Next: generate the SDK TypeScript types from the same schema and retire the + hand-maintained `*-strict.json` stable view. diff --git a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md index ed766a1f4..a81976683 100644 --- a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md +++ b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md @@ -42,7 +42,7 @@ on the response, and neither shape carries `containerId`. |---|---|---| | TypeScript SDK (reference §6) | Five new functions: `provisionSandbox`, `startSandbox`, `execInSandbox` / `execInSandboxAsync`, `stopSandbox`, `deprovisionSandbox`. Branded `SandboxId` type tagging ids by backend (`containment` named once at provision, inferred from the id thereafter). Per-(backend, phase) typed `*Config` interfaces (e.g. `IsolationSessionProvisionConfig`) that absorb cross-cutting fields directly — no separate policy parameter. Per-phase typed `*Result` types per backend. `AbortSignal` cancellation via the existing `SandboxSpawnOptions`. Typed `MxcError` class carrying a closed-enum `code`. | `spawnSandbox` family preserved. `ContainmentBackend` extension reused. The wire-format-aligned `Process` / `Filesystem` / `Network` / `UiConfig` interfaces from `sdk/src/types.ts` are reused as field types inside state-aware Configs. `SandboxSpawnOptions` reused as the third-arg options bag (gains `signal?: AbortSignal`). `*Config` naming convention reused. | | JSON wire format (reference §7) | Top-level `phase` discriminator. Top-level `sandboxId`. `containment` carried on provision only; non-provision phases route via the `sandboxId` prefix. Per-phase nesting under `experimental..`. Named envelope types as a TypeScript discriminated union. | One-shot configs (no `phase`) work unchanged. Cross-cutting `filesystem` / `network` / `ui` at top level for state-aware too — backends declare per-phase honor. | -| Rust executor (reference §9) | Dispatch arm for state-aware. New `StatefulSandboxBackend` trait. Rust mirror of the wire envelope (private `Raw*` parser pattern). | `ScriptRunner` trait. Existing one-shot dispatch path. Existing backends unchanged. | +| Rust executor (reference §9) | Dispatch arm for state-aware. New `StatefulSandboxBackend` trait. Rust mirror of the wire envelope (the `wire::MxcConfig` parse target). | `ScriptRunner` trait. Existing one-shot dispatch path. Existing backends unchanged. | | Error model (reference §8) | Closed enum of 12 codes. `MxcError` class with `code: ErrorCode`. `details` open object. | Existing one-shot error paths preserved. | | Plug-in surface (reference §11) | Implement `StatefulSandboxBackend`. Define typed per-(backend, phase) `*Config` interfaces. Declare the trait's `ID_PREFIX` and `BACKEND_KEY` consts. Document the cross-cutting honor matrix. | Ephemeral-only backends require no changes. | @@ -141,8 +141,8 @@ state-aware Config's `filesystem` field — no change to the helpers. ## Wire contract The wire envelope is a TypeScript discriminated union over `phase`, JSON-serialised. -The Rust executor parses the same shape via private `Raw*` intermediate structs -(reference §9.1). The only `Record` in the contract is +The Rust executor parses the same shape into the typed wire model +(`wire::MxcConfig`, reference §9.1). The only `Record` in the contract is `ErrorEnvelope.details` — the escape hatch for backend-specific structured failure information. @@ -407,8 +407,8 @@ Reference §11 has the full guide. Operational checklist: `BACKEND_KEY` (the wire-format `containment` value, used for provision-phase routing and `experimental..` deserialisation). Add a dispatch arm for the new variant. -5. Add `Raw*` intermediate structs in `config_parser.rs` for the backend's wire-format - block. +5. Add typed fields to the `experimental` block of the wire model (`wire.rs`) for the + backend's wire-format block, then regenerate the schema. 6. Document policy-honor matrix, idempotence, concurrency, and error mapping in `docs//.md` (e.g., `docs/isolation-session/state-aware-plan.md`). diff --git a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md index de7fddb5a..4445fba3a 100644 --- a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md +++ b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md @@ -65,7 +65,7 @@ elaborates. |---|---|---| | TypeScript SDK (§6) | Five new functions: `provisionSandbox`, `startSandbox`, `execInSandbox` / `execInSandboxAsync`, `stopSandbox`, `deprovisionSandbox`. Branded `SandboxId` type tagging ids by backend (`containment` named once at provision, inferred from the id thereafter). Per-(backend, phase) typed `*Config` interfaces (e.g. `IsolationSessionProvisionConfig`) that absorb cross-cutting fields directly — no separate policy parameter. Per-phase typed `*Result` types per backend. `AbortSignal` cancellation via the existing `SandboxSpawnOptions`. Typed `MxcError` class carrying a closed-enum `code`. | `spawnSandbox` family preserved. `ContainmentBackend` extension mechanism reused. The existing wire-format-aligned `ProcessConfig` / `FilesystemConfig` / `NetworkConfig` / `UiConfig` interfaces from `sdk/src/types.ts` are reused as field types inside the new state-aware Configs. `SandboxSpawnOptions` reused as the third-arg options bag (gains `signal?: AbortSignal`). Existing typed `*Config` naming convention reused. | | JSON wire format (§7) | Top-level `phase` discriminator. Top-level `sandboxId`. `containment` carried on provision only; non-provision phases route via the `sandboxId` prefix. Per-phase nesting under `experimental..`. Named envelope types as a TypeScript discriminated union over `phase`. | One-shot configs (no `phase`) work unchanged. Cross-cutting `filesystem` / `network` / `ui` fields at top level for state-aware too — backends declare per-phase honor. | -| Rust executor (§9) | Dispatch arm for state-aware. New `StatefulSandboxBackend` trait. Rust mirror of the wire envelope (private to parser, matching `RawConfig` pattern). | `ScriptRunner` trait. Existing one-shot dispatch path. Existing backends function without modification. | +| Rust executor (§9) | Dispatch arm for state-aware. New `StatefulSandboxBackend` trait. Rust mirror of the wire envelope (the `wire::MxcConfig` parse target). | `ScriptRunner` trait. Existing one-shot dispatch path. Existing backends function without modification. | | Error model (§8) | Closed enum of 12 error codes. `MxcError` class with `code: ErrorCode`. `details` open object as escape hatch for backend-specific structured information. | Existing one-shot error paths preserved. | | Plug-in surface (§11) | Implement `StatefulSandboxBackend` (in addition to or instead of `ScriptRunner`). Define typed per-(backend, phase) `*Config` interfaces. Declare the backend's `ID_PREFIX` and `BACKEND_KEY` consts on the trait impl. Document the cross-cutting policy honor matrix. | Ephemeral-only backends require no changes. The `ContainmentBackend` Rust enum is extended, not replaced. | @@ -982,70 +982,54 @@ implements one trait, the other, or both, depending on its declared participatio ### 9.1 Wire envelope (Rust mirror) -MXC's existing parser at `src/core/wxc_common/src/config_parser.rs` uses private `Raw*` -intermediate structs that mirror the wire-format JSON shape (with serde renames to handle -camelCase keys), then converts them into typed domain models via `convert_*` helpers -(e.g., `RawConfig` → `ExecutionRequest`) before dispatch. The state-aware path extends this -same pattern. +MXC's parser at `src/core/wxc_common/src/config_parser.rs` deserializes the +wire-format JSON directly into the typed wire model in +`src/core/wxc_common/src/wire.rs` (`wire::MxcConfig`), then maps it into the +typed domain models via `convert_*` helpers (`convert_wire_config` → +`ExecutionRequest`) before dispatch. The state-aware path reuses this same wire +model. ```rust -// In config_parser.rs — private to the parser, alongside the existing Raw* structs. -#[derive(Deserialize)] -#[serde(untagged)] -enum RawMxcRequest { - StateAware(RawStateAwareRequest), - OneShot(RawConfig), -} - -#[derive(Deserialize)] -struct RawStateAwareRequest { - version: Option, - containment: Option, - phase: String, - #[serde(rename = "sandboxId")] - sandbox_id: Option, - process: Option, - filesystem: Option, - network: Option, - ui: Option, - experimental: Option, -} - -#[derive(Deserialize)] -struct RawStateAwareExperimental { - #[serde(rename = "isolation_session")] - isolation_session: Option, - // future state-aware-capable backends add typed entries here -} - -#[derive(Deserialize)] -struct RawIsolationSessionConfigs { - start: Option, - // omit phases without config +// In config_parser.rs — discrimination is by presence of the `phase` key in +// the decoded JSON; both shapes deserialize into the one wire::MxcConfig type, +// which declares `phase` / `sandboxId` and the per-backend `experimental` block. +let value: serde_json::Value = serde_json::from_str(&json_str)?; +if value.get("phase").is_some() { + // state-aware: peel off the raw `experimental` block (typed per-backend at + // dispatch), deserialize the rest into wire::MxcConfig, then map. + convert_wire_state_aware(value, logger, allow_missing_command) +} else { + // one-shot: deserialize wire::MxcConfig from the source text (preserving + // serde line/column diagnostics) and map. + let cfg: wire::MxcConfig = serde_json::from_str(&json_str)?; + convert_wire_config(cfg, logger, true, allow_missing_command) } ``` -Discrimination is by presence of the `phase` field. With `#[serde(untagged)]`, serde -attempts each variant in order; the state-aware variant requires `phase`, so its absence -falls through to the one-shot branch. Per-phase requirements (`containment` for -`provision`, `sandboxId` for the others) are enforced in the conversion step rather -than at the deserialiser; the `Raw*` struct accepts both fields as optional. - -Conversion from `Raw*` into the typed domain model is the existing -`convert_raw_config` → `ExecutionRequest` path, used for both one-shot and state-aware -requests. The cross-cutting wire fields (`filesystem`, `network`, `ui`) populate -`ExecutionRequest.policy` (a `ContainerPolicy`) exactly as the one-shot path does today, -and `process` populates `ExecutionRequest`'s flat `script_code` / `working_directory` / -`script_timeout` / `env` fields via the existing `RawProcess` intermediate. The -state-aware-only fields (`phase`, `sandboxId`, `experimental..`) are -extracted alongside the `ExecutionRequest` and bundled into a `ParsedStateAwareRequest` -domain model — `{ request: ExecutionRequest, phase: Phase, containment: -Option, sandbox_id: Option, experimental_raw: ... }` — -that the dispatcher consumes (§9.3). The bundling does not modify `ExecutionRequest`'s -shape. There is no unified Rust "policy" type — Rust reads cross-cutting fields -directly from `request.policy` (a `ContainerPolicy`), exactly as the one-shot path does -today. Domain models are exposed to the dispatch layer; the `Raw*` types stay private -to the parser. +`wire::MxcConfig` is closed (`deny_unknown_fields`) on its stable surface, so +unknown fields are rejected at the trust boundary. `phase` maps to the +`wire::Phase` enum. The `experimental` block stays permissive and is captured as +a raw `serde_json::Value` on the state-aware path so the dispatcher can type each +backend's per-phase config from it (`experimental..`). + +Per-phase requirements (`containment` for `provision`, `sandboxId` for the +others) are enforced in the conversion step, not at the deserializer. The +one-shot path rejects `phase` / `sandboxId`, and the state-aware path rejects +one-shot-only sections (`seatbelt` / `processContainer` / `lxc` / `lifecycle`), +so each mode only accepts its valid fields. + +Conversion populates the cross-cutting wire fields (`filesystem`, `network`, +`ui`) into `ExecutionRequest.policy` (a `ContainerPolicy`) exactly as the +one-shot path does, and `process` populates `ExecutionRequest`'s flat +`script_code` / `working_directory` / `script_timeout` / `env` fields. The +state-aware-only fields (`phase`, `sandboxId`, `experimental..`) +are extracted alongside the `ExecutionRequest` and bundled into a +`ParsedStateAwareRequest` domain model — `{ request: ExecutionRequest, phase: +Phase, containment: Option, sandbox_id: Option, +experimental_raw: Option }` — that the dispatcher consumes +(§9.3). The bundling does not modify `ExecutionRequest`'s shape. Domain models +are exposed to the dispatch layer; the wire types are an implementation detail of +the parser and schema generation. ### 9.2 The trait @@ -1215,7 +1199,7 @@ pub struct ExecHandle { ``` Trait methods take `&ExecutionRequest` (the existing one-shot domain model from -`wxc_common::models`, populated by the same `convert_raw_config` parser path that +`wxc_common::models`, populated by the same `convert_wire_config` parser path that serves one-shot calls), plus `sandbox_id` for non-provision phases and an optional backend-specific typed config (`Self::Config`). Cross-cutting policy fields flow through `request.policy` (a `ContainerPolicy`); per-exec process info flows @@ -1619,10 +1603,9 @@ capability mismatches automatically (§9.4). ### 11.5 Add a config-parser case The state-aware wire format expects `experimental..` blocks for backends -that declare per-phase configs. Add typed `Raw*` intermediate structs in -`config_parser.rs` (matching the existing `RawConfig` pattern) that deserialise the new -backend's JSON shape. Add a converter that produces the typed domain models the -dispatch layer consumes. +that declare per-phase configs. Add typed fields to the `experimental` block of the wire +model (`wire.rs`) for the new backend's JSON shape, then regenerate the schema. Add a +converter that produces the typed domain models the dispatch layer consumes. ### 11.6 Document the backend diff --git a/docs/versioning.md b/docs/versioning.md index b6d1f1ec6..19ed2abc2 100644 --- a/docs/versioning.md +++ b/docs/versioning.md @@ -104,11 +104,13 @@ lxc-exec config.json --experimental wxc-exec.exe --experimental config.json ``` -When `--experimental` is passed: -- The parser reads the `experimental` section from the config JSON -- Features from the experimental section are applied alongside the stable features -- Without the flag, the `experimental` section is **silently ignored** — no error, - just skipped +The parser **always** parses and preserves the `experimental` section regardless +of the flag; parsing is flag-independent. The `--experimental` flag only sets +`request.experimental_enabled`: +- When set, the runners apply the parsed experimental features alongside the + stable features +- When unset, `experimental_enabled` is false and the runners **ignore** the + parsed experimental section — no error, the features are just not applied **2. SDK (`@microsoft/mxc-sdk`):** ```typescript @@ -134,21 +136,26 @@ The SDK passes `--experimental` to the underlying binary when this option is set Developers adding experimental features follow this pattern. For a detailed step-by-step guide, see [Authoring a New Feature](authoring-a-new-feature.md). -**In `config_parser.rs`:** +**In `wire.rs` (the parse + schema source of truth):** ```rust -struct RawConfig { +pub struct MxcConfig { // ... stable fields ... - experimental: Option, + pub experimental: Option, } -struct RawExperimental { - compartments: Option, - #[serde(rename = "gpuIsolation")] - gpu_isolation: Option, +// The `experimental` block is intentionally permissive (no deny_unknown_fields) +// so in-flux feature shapes stay forward-compatible. +pub struct Experimental { + pub compartments: Option, + pub gpu_isolation: Option, // ... add new experimental features here ... } ``` +After editing `wire.rs`, regenerate the schema +(`cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- schemas/dev/mxc-config.schema.0.8.0-dev.json`) +— do not hand-edit the generated schema. + **In `models.rs`:** ```rust pub struct ExperimentalConfig { @@ -163,6 +170,9 @@ pub struct ExecutionRequest { } ``` +**In `config_parser.rs`:** map the wire `Experimental` field to the domain +`ExperimentalConfig` inside `convert_wire_config` (there is no `Raw*` struct). + **In the runner (e.g., `appcontainer.rs`):** ```rust fn run(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResponse { @@ -181,10 +191,13 @@ fn run(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResp ``` **Promotion process:** When an experimental feature is ready to ship: -1. Move the property from `experimental` to a top-level property in the schema - (e.g., `experimental.gpuIsolation` → `gpuIsolation`) +1. Move the field from the wire `Experimental` struct to top-level `MxcConfig` + (e.g., `experimental.gpuIsolation` → top-level `gpuIsolation`), then + regenerate the schema with `mxc_schema_gen` 2. Move the struct from `ExperimentalConfig` to `ExecutionRequest` -3. Move the field from `RawExperimental` to `RawConfig` +3. Map the now-top-level wire field in `convert_wire_config`; add + `deny_unknown_fields` to the wire struct so the promoted stable surface is + closed 4. Remove the `if request.experimental_enabled` guard 5. Bump the minor version 6. Add a parser error for configs still referencing the feature under @@ -199,41 +212,16 @@ fn run(&mut self, request: &ExecutionRequest, logger: &mut Logger) -> ScriptResp - In `wxc_common::config_parser`, rename the matching entry in `present_backend_sections` and `owned_backend_section` from `experimental.` to ``. - - In the JSON schema's top-level `allOf`, rekey the matching `if/then` - clause so it checks the new top-level section instead of - `experimental.`. - - Concretely, if `wslc` graduates the clause changes from - - ```json - { - "if": { - "required": ["experimental"], - "properties": { "experimental": { "required": ["wslc"] } } - }, - "then": { - "required": ["containment"], - "properties": { "containment": { "enum": ["wslc"] } } - } - } - ``` - - to - - ```json - { - "if": { "required": ["wslc"] }, - "then": { - "required": ["containment"], - "properties": { "containment": { "enum": ["wslc"] } } - } - } - ``` - - The `then` branch is unchanged: a backend section requires `containment` - to be set, and the value must be either the concrete backend name or any - abstract intent that resolves to it on at least one platform (for - example, `processContainer` accepts both `processcontainer` and `process`). + + The single-backend-section rule is a cross-field constraint enforced by the + parser (the trust boundary), **not** by the JSON schema — the generated + schema intentionally omits the old top-level `allOf` `if/then` clauses. So + there is no schema edit for this step; the parser change is sufficient. + + The rule itself is unchanged: a backend section requires `containment` to be + set, and the value must be either the concrete backend name or any abstract + intent that resolves to it on at least one platform (for example, + `processContainer` accepts both `processcontainer` and `process`). ## Data Flow @@ -355,11 +343,15 @@ mirrors that behavior. We do *not* gate alias acceptance on schema version (i.e. removal in a future minor release; gating buys little and costs review complexity in every layer that re-checks containment. -**Observability.** Each legacy-value encounter emits a one-line deprecation hint -via the existing diagnostic channel (Rust: `Logger`; TypeScript SDK: `diagLog`, -dedup'd per legacy value per process). The hint names the canonical replacement. -No throw, no stderr write — the deprecation is observable only to callers who -opt into the diagnostic stream. +**Observability.** Legacy *value* aliases are mapped to their canonical form by +serde (`#[serde(alias = "...")]`) during deserialization, so the Rust parser no +longer emits a per-value deprecation hint for them — serde normalizes the alias +before any parser code runs, and the wire model is the trust boundary. Aliases +are still accepted; they are simply silent in the native parser. The TypeScript +SDK validator may still surface a deprecation hint via `diagLog` where it +inspects the raw config before serialization. (Earlier revisions emitted a +`Logger` line from the hand-written parser for each legacy value; that path was +removed when the parser was rewired onto the wire model.) **Removal.** When an alias is removed in a future release, the change goes through the same promotion-style migration: a single release that turns the diff --git a/schemas/dev/mxc-config.schema.0.8.0-dev.json b/schemas/dev/mxc-config.schema.0.8.0-dev.json index a87df0ed0..d0ed5ffcb 100644 --- a/schemas/dev/mxc-config.schema.0.8.0-dev.json +++ b/schemas/dev/mxc-config.schema.0.8.0-dev.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://github.com/microsoft/mxc/schemas/dev/mxc-config.schema.0.8.0-dev.json", "title": "MXC Configuration", - "description": "MXC container execution configuration. Defines the recommended config format for both one-shot and state-aware sandbox lifecycle requests. The parser also accepts legacy fields not listed here during the migration period.", + "description": "MXC container execution configuration. Defines the recommended config format for both one-shot and state-aware sandbox lifecycle requests. A few deprecated field spellings not listed here are also accepted via serde aliases.", "additionalProperties": false, "definitions": { "BaseProcessUi": { @@ -359,8 +359,7 @@ "type": "object" }, "IsolationUser": { - "additionalProperties": false, - "description": "Entra cloud-agent user bundle.", + "description": "Entra cloud-agent user bundle. Reachable only under the permissive `experimental` surface, so unknown fields are tolerated (forward-compat).", "properties": { "upn": { "description": "User principal name.", @@ -551,8 +550,7 @@ "type": "string" }, "PortMapping": { - "additionalProperties": false, - "description": "A single host → container port forward.", + "description": "A single host → container port forward. Reachable only under the permissive `experimental` surface, so unknown fields are tolerated (forward-compat).", "properties": { "containerPort": { "description": "Container port.", diff --git a/sdk/src/sandbox.ts b/sdk/src/sandbox.ts index 19e6b63a6..6c730deb2 100644 --- a/sdk/src/sandbox.ts +++ b/sdk/src/sandbox.ts @@ -105,13 +105,13 @@ function buildBubblewrapConfig( */ function buildLinuxProcessConfig( config: ContainerConfig, - containerId: string, ): ContainerConfig { + // The container id is carried at top-level `containerId` and the teardown + // flag at `lifecycle.destroyOnExit`; the wire `lxc` object has only + // `distribution` / `release`, so do not duplicate those here. config.lxc = { - containerName: containerId, distribution: 'alpine', release: '3.23', - destroyOnExit: true, }; applyLinuxNetworkPolicy(config); return config; @@ -140,7 +140,6 @@ function buildDarwinProcessConfig( function buildProcessBaseContainerConfig( config: ContainerConfig, policy: SandboxPolicy, - containerId: string, ): ContainerConfig { const capabilities: string[] = []; if (policy.network?.allowOutbound) { @@ -150,8 +149,10 @@ function buildProcessBaseContainerConfig( capabilities.push("privateNetworkClientServer"); } + // The container id is carried only at the top level (`containerId`, set by + // createConfigFromPolicy); the wire `processContainer` object has no `name` + // field, so emitting it here would be rejected by the parser. config.processContainer = { - name: containerId, leastPrivilege: false, capabilities, ui: { @@ -198,7 +199,6 @@ function buildMicroVmConfig( readwritePaths: policy.filesystem?.readwritePaths, readonlyPaths: policy.filesystem?.readonlyPaths, deniedPaths: policy.filesystem?.deniedPaths, - clearPolicyOnExit: policy.filesystem?.clearPolicyOnExit ?? true, }; } config.containment = 'microvm'; @@ -344,7 +344,7 @@ export function createConfigFromPolicy( if (containment === 'lxc') { diagLog(`createConfigFromPolicy: containment=lxc, id=${containerId}`); config.containment = 'lxc'; - return buildLinuxProcessConfig(config, containerId); + return buildLinuxProcessConfig(config); } if (containment === 'process') { @@ -369,7 +369,7 @@ export function createConfigFromPolicy( return buildDarwinProcessConfig(config); } diagLog(`createConfigFromPolicy: containment=process (BaseContainer), id=${containerId}`); - return buildProcessBaseContainerConfig(config, policy, containerId); + return buildProcessBaseContainerConfig(config, policy); } throw new Error(`Containment type '${containment}' is not yet supported.`); diff --git a/sdk/tests/unit/sandbox.test.ts b/sdk/tests/unit/sandbox.test.ts index 1fd5accd7..375c1cdc9 100644 --- a/sdk/tests/unit/sandbox.test.ts +++ b/sdk/tests/unit/sandbox.test.ts @@ -312,7 +312,7 @@ describe('buildSandboxPayload', () => { } }); - it('should include filesystem with clearPolicyOnExit for microvm when policy has paths', () => { + it('should map clearPolicyOnExit to lifecycle.preservePolicy for microvm when policy has paths', () => { mockWindows(); try { const policy: SandboxPolicy = { @@ -322,13 +322,15 @@ describe('buildSandboxPayload', () => { const payload = buildSandboxPayload('print(42)', policy, undefined, undefined, 'microvm'); assert.strictEqual(payload.containment, 'microvm'); assert.deepStrictEqual(payload.filesystem!.readwritePaths, ['/tmp']); - assert.strictEqual(payload.filesystem!.clearPolicyOnExit, true); + // clearPolicyOnExit is not a wire `filesystem` field; the intent is + // carried canonically by lifecycle.preservePolicy (default clear => not preserved). + assert.strictEqual(payload.lifecycle!.preservePolicy, false); } finally { restore(); } }); - it('should honor clearPolicyOnExit false for microvm', () => { + it('should honor clearPolicyOnExit false for microvm (via lifecycle.preservePolicy)', () => { mockWindows(); try { const policy: SandboxPolicy = { @@ -336,7 +338,7 @@ describe('buildSandboxPayload', () => { filesystem: { readwritePaths: ['/tmp'], clearPolicyOnExit: false }, }; const payload = buildSandboxPayload('print(42)', policy, undefined, undefined, 'microvm'); - assert.strictEqual(payload.filesystem!.clearPolicyOnExit, false); + assert.strictEqual(payload.lifecycle!.preservePolicy, true); } finally { restore(); } diff --git a/src/core/mxc-sdk/src/policy.rs b/src/core/mxc-sdk/src/policy.rs index f5d0b69a1..703cfc4a1 100644 --- a/src/core/mxc-sdk/src/policy.rs +++ b/src/core/mxc-sdk/src/policy.rs @@ -760,8 +760,10 @@ fn apply_backend(config: &mut serde_json::Value, policy: &SandboxPolicy, contain capabilities.push("privateNetworkClientServer"); } } + // The container id is carried only at the top level (`containerId`); the + // wire `processContainer` object intentionally has no `name` field. + let _ = container_id; config["processContainer"] = json!({ - "name": container_id, "leastPrivilege": false, "capabilities": capabilities, "ui": { diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index de72c81d7..51348d651 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -1,24 +1,21 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::collections::HashMap; use std::fmt::Write; use std::fs; -use serde::de::IgnoredAny; -use serde::Deserialize; - use crate::encoding::base64_decode; use crate::error::WxcError; use crate::logger::Logger; use crate::models::{ ClipboardPolicy, ContainerPolicy, ContainmentBackend, ExecutionRequest, ExperimentalConfig, - IsolationSessionConfig, IsolationSessionUser, LifecycleConfig, LxcConfig, - NetworkEnforcementMode, NetworkPolicy, PortMapping, ProxyAddress, ProxyConfig, SeatbeltConfig, - TestFeatureConfig, UiPolicy, WindowsSandboxConfig, WslcConfig, + IsolationSessionConfig, IsolationSessionConfigurationId, IsolationSessionUser, LifecycleConfig, + LxcConfig, NetworkEnforcementMode, NetworkPolicy, PortMapping, ProxyAddress, ProxyConfig, + SeatbeltConfig, TestFeatureConfig, UiPolicy, WindowsSandboxConfig, WslcConfig, }; use crate::mxc_error::MxcError; use crate::state_aware_request::{MxcRequest, ParsedStateAwareRequest, Phase}; +use crate::wire; /// Categorised error from `load_mxc_request`. The `wxc-exec` driver uses the /// variant to choose the failure-output convention: state-aware failures @@ -36,343 +33,8 @@ pub enum ParseError { StateAware(MxcError), } -// ---------- Intermediate serde structs matching the JSON schema ---------- - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawProcessContainer { - #[serde(rename = "leastPrivilege")] - least_privilege: Option, - #[serde(rename = "learningMode")] - learning_mode: Option, - capabilities: Option>, - ui: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawBaseProcessUi { - isolation: Option, - #[serde(rename = "desktopSystemControl")] - desktop_system_control: Option, - #[serde(rename = "systemSettings")] - system_settings: Option, - ime: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawFilesystem { - #[serde(rename = "readwritePaths")] - readwrite_paths: Option>, - #[serde(rename = "readonlyPaths")] - readonly_paths: Option>, - #[serde(rename = "deniedPaths")] - denied_paths: Option>, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawFallback { - #[serde(rename = "allowDaclMutation")] - allow_dacl_mutation: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawNetwork { - #[serde(rename = "defaultPolicy")] - default_policy: Option, - #[serde(rename = "enforcementMode")] - enforcement_mode: Option, - #[serde(rename = "allowLocalNetwork")] - allow_local_network: Option, - #[serde(rename = "allowedHosts")] - allowed_hosts: Option>, - #[serde(rename = "blockedHosts")] - blocked_hosts: Option>, - proxy: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawSandbox { - #[serde(rename = "idleTimeout")] - idle_timeout: Option, - #[serde(rename = "idleTimeoutMs")] - idle_timeout_ms: Option, - #[serde(rename = "daemonPipeName")] - daemon_pipe_name: Option, -} - -#[derive(Deserialize)] -struct RawPortMapping { - #[serde(rename = "windowsPort")] - windows_port: u16, - #[serde(rename = "containerPort")] - container_port: u16, - protocol: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawContainerConfig { - #[serde(rename = "targetOs")] - target_os: Option, - image: Option, - #[serde(rename = "imageTarPath")] - image_tar_path: Option, - #[serde(rename = "cpuCount")] - cpu_count: Option, - #[serde(rename = "memoryMb")] - memory_mb: Option, - gpu: Option, - #[serde(rename = "storagePath")] - storage_path: Option, - #[serde(rename = "portMappings")] - port_mappings: Option>, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawLxc { - distribution: Option, - release: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawProcess { - #[serde(rename = "commandLine")] - command_line: Option, - cwd: Option, - env: Option>, - timeout: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawLifecycle { - #[serde(rename = "destroyOnExit")] - destroy_on_exit: Option, - #[serde(rename = "preservePolicy")] - preserve_policy: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawTestFeature { - message: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawIsolationSession { - #[serde(rename = "configurationId")] - configuration_id: Option, - user: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawSeatbelt { - #[serde(rename = "profileOverride")] - profile_override: Option, - #[serde(rename = "guiAccess")] - gui_access: Option, - #[serde(rename = "launchMethod")] - launch_method: Option, - #[serde(rename = "nestedPty")] - nested_pty: Option, - #[serde(rename = "keychainAccess")] - keychain_access: Option, - #[serde(rename = "extraMachLookups")] - extra_mach_lookups: Option>, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawExperimental { - test: Option, - #[serde(rename = "windows_sandbox")] - windows_sandbox: Option, - wslc: Option, - #[serde(rename = "isolation_session")] - isolation_session: Option, - #[serde(alias = "macos_sandbox")] - seatbelt: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawUi { - disable: Option, - clipboard: Option, - injection: Option, -} - -#[derive(Deserialize, Default)] -#[serde(default)] -struct RawConfig { - version: Option, - #[serde(rename = "containerId")] - container_id: Option, - process: Option, - lifecycle: Option, - containment: Option, - #[serde(rename = "processContainer", alias = "appContainer")] - process_container: Option, - lxc: Option, - filesystem: Option, - fallback: Option, - network: Option, - ui: Option, - experimental: Option, - /// Top-level seatbelt config. - #[serde(alias = "macos_sandbox")] - seatbelt: Option, - // Captures any top-level key not matched above so unrecognised fields can - // be rejected at the trust boundary instead of being silently dropped. - // `IgnoredAny` discards each value during deserialisation, so a large or - // complex unknown section costs nothing beyond its key. Allowed annotation - // keys ($schema, _comment) are filtered out in - // `reject_unknown_top_level_fields`. - #[serde(flatten)] - unknown: HashMap, -} - -// State-aware request shape. `phase` is required (no `#[serde(default)]` on -// the struct, no field-level default) and acts as the discriminator against -// `RawConfig`; the other fields mirror `RawConfig`'s wire shape so -// cross-cutting fields (filesystem/network/ui/process) populate the inner -// `ExecutionRequest` via the same conversion path. The `experimental` block stays -// raw — typed deserialisation happens at dispatch time keyed by backend. -#[derive(Deserialize)] -struct RawStateAwareRequest { - #[serde(default)] - version: Option, - #[serde(rename = "containerId", default)] - container_id: Option, - #[serde(default)] - containment: Option, - phase: String, - #[serde(rename = "sandboxId", default)] - sandbox_id: Option, - #[serde(default)] - process: Option, - #[serde(default)] - filesystem: Option, - #[serde(default)] - fallback: Option, - #[serde(default)] - network: Option, - #[serde(default)] - ui: Option, - #[serde(default)] - experimental: Option, - // See `RawConfig::unknown`. Mirrored here so unrecognised top-level keys are - // rejected on the state-aware path too. - #[serde(flatten)] - unknown: HashMap, -} - -// Untagged enum: serde tries `StateAware` first (requires `phase`), falls -// through to `OneShot` when `phase` is absent. Order matters — `OneShot` is -// permissive enough to accept arbitrary wire shapes. Both variants are boxed -// so the enum stays a single tagged pointer regardless of inner growth. -#[derive(Deserialize)] -#[serde(untagged)] -enum RawMxcRequest { - StateAware(Box), - OneShot(Box), -} - // ---------- Public API ---------- -/// Parse the `proxy` field. -/// -/// Accepts either `{ "localhost": }` for an external localhost proxy, -/// `{ "builtinTestServer": true }` to have wxc launch its own test proxy, -/// or `{ "url": "" }` for a proxy URL (parsed into host:port). -/// When `builtinTestServer` is set it must be the only key in the object. -fn parse_proxy_config(value: &serde_json::Value) -> Result { - let obj = value - .as_object() - .ok_or_else(|| WxcError::ConfigParse("network.proxy must be an object".to_string()))?; - - let mut proxy_addr = ProxyAddress::new("127.0.0.1".to_string(), 0); - - if let Some(builtin_value) = obj.get("builtinTestServer") { - if builtin_value.as_bool() != Some(true) { - return Err(WxcError::ConfigParse( - "network.proxy.builtinTestServer must be true when present".to_string(), - )); - } - if obj.len() != 1 { - return Err(WxcError::ConfigParse( - "When builtinTestServer is true, no other proxy options may be set".to_string(), - )); - } - - return Ok(ProxyConfig { - address: Some(proxy_addr), - builtin_test_server: true, - }); - } - - if let Some(localhost) = obj.get("localhost") { - let port_val = if let Some(port) = localhost.as_u64() { - port - } else { - return Err(WxcError::ConfigParse( - "network.proxy.localhost must be a number".to_string(), - )); - }; - - if port_val == 0 || port_val > 65535 { - return Err(WxcError::ConfigParse( - "network.proxy.localhost must be a port between 1 and 65535".to_string(), - )); - } - - // Non builtin proxy with localhost and port specified - proxy_addr.port = port_val as u16; - return Ok(ProxyConfig { - address: Some(proxy_addr), - builtin_test_server: false, - }); - } - - if let Some(url_value) = obj.get("url") { - let url_str = url_value.as_str().ok_or_else(|| { - WxcError::ConfigParse("network.proxy.url must be a string".to_string()) - })?; - - let parsed = url::Url::parse(url_str) - .map_err(|e| WxcError::ConfigParse(format!("network.proxy.url is invalid: {e}")))?; - - let host = parsed.host_str().ok_or_else(|| { - WxcError::ConfigParse(format!( - "network.proxy.url must include a host (e.g., http://localhost:8080), got: {url_str}" - )) - })?.to_string(); - let port = parsed.port().ok_or_else(|| { - WxcError::ConfigParse(format!( - "network.proxy.url must include a port (e.g., http://localhost:8080), got: {url_str}" - )) - })?; - - return Ok(ProxyConfig { - address: Some(ProxyAddress::from_url(url_str, host, port)), - builtin_test_server: false, - }); - } - - Err(WxcError::ConfigParse( - "network.proxy must specify builtinTestServer, localhost, or url".to_string(), - )) -} - /// Options for [`load_mxc_request_with_options`]. /// /// Kept as a struct (rather than additional positional arguments) so future @@ -419,12 +81,12 @@ pub fn load_request_with_options( ) -> Result { let json_str = decode_request_input(input, logger, opts.is_base64)?; - let raw: RawConfig = serde_json::from_str(&json_str).map_err(|e| { + let cfg: wire::MxcConfig = serde_json::from_str(&json_str).map_err(|e| { logger.log_line("Error parsing JSON"); WxcError::ConfigParse(format!("JSON parse error: {}", e)) })?; - convert_raw_config_inner(raw, logger, true, opts.allow_missing_command) + convert_wire_config(cfg, logger, true, opts.allow_missing_command) } /// Build a request from an already-parsed wire-format config [`Value`], running @@ -439,12 +101,12 @@ pub fn load_request_from_value( logger: &mut Logger, allow_missing_command: bool, ) -> Result { - let raw: RawConfig = serde_json::from_value(config).map_err(|e| { + let cfg: wire::MxcConfig = serde_json::from_value(config).map_err(|e| { logger.log_line("Error parsing JSON"); WxcError::ConfigParse(format!("JSON parse error: {}", e)) })?; - convert_raw_config_inner(raw, logger, true, allow_missing_command) + convert_wire_config(cfg, logger, true, allow_missing_command) } /// driver can pick the right output convention per path (envelope on stdout /// for state-aware, diagnostic on stderr for one-shot and pre-discrimination @@ -476,22 +138,31 @@ pub fn load_mxc_request_with_options( let json_str = decode_request_input(input, logger, opts.is_base64).map_err(ParseError::Decode)?; - let raw: RawMxcRequest = serde_json::from_str(&json_str).map_err(|e| { + // Parse once into a generic value so we can (a) discriminate one-shot vs + // state-aware by presence of the `phase` key and (b) capture the raw + // `experimental` block for the state-aware path, where it is typed + // per-backend at dispatch time rather than at parse time. + let value: serde_json::Value = serde_json::from_str(&json_str).map_err(|e| { logger.log_line("Error parsing JSON"); ParseError::Decode(WxcError::ConfigParse(format!("JSON parse error: {}", e))) })?; - match raw { - RawMxcRequest::StateAware(state_aware) => { - convert_raw_state_aware(*state_aware, logger, opts.allow_missing_command) - .map(MxcRequest::StateAware) - .map_err(|e| ParseError::StateAware(MxcError::malformed_request(e.to_string()))) - } - RawMxcRequest::OneShot(one_shot) => { - convert_raw_config_inner(*one_shot, logger, true, opts.allow_missing_command) - .map(MxcRequest::OneShot) - .map_err(ParseError::OneShot) - } + if value.get("phase").is_some() { + convert_wire_state_aware(value, logger, opts.allow_missing_command) + .map(MxcRequest::StateAware) + .map_err(|e| ParseError::StateAware(MxcError::malformed_request(e.to_string()))) + } else { + // Re-deserialize from the source text (not the parsed `Value`) so + // serde's line/column context is preserved in error messages on this + // trust boundary; `from_value` discards it, turning a typo or + // out-of-range field into an unlocalised "expected u16"-style dump. + let cfg: wire::MxcConfig = serde_json::from_str(&json_str).map_err(|e| { + logger.log_line("Error parsing JSON"); + ParseError::OneShot(WxcError::ConfigParse(format!("JSON parse error: {}", e))) + })?; + convert_wire_config(cfg, logger, true, opts.allow_missing_command) + .map(MxcRequest::OneShot) + .map_err(ParseError::OneShot) } } @@ -629,59 +300,95 @@ fn validate_paths(paths: &[String], logger: &mut Logger) -> Result<(), WxcError> Ok(()) } -// ---------- Conversion from raw JSON to domain model ---------- +// ---------- Conversion from wire model to domain model ---------- + +/// Convert a typed `wire::Proxy` block into the validated domain `ProxyConfig`. +/// Exactly one of `builtinTestServer` / `localhost` / `url` may be set. +fn convert_wire_proxy(proxy: wire::Proxy) -> Result { + // Destructure (no `..`) so a new wire field fails to compile until handled. + let wire::Proxy { + builtin_test_server, + localhost, + url, + } = proxy; + let mut proxy_addr = ProxyAddress::new("127.0.0.1".to_string(), 0); -/// Top-level keys that are permitted but carry no semantic meaning: `$schema` -/// (editor hint) and `_comment` (human annotation). They are accepted at the -/// trust boundary but ignored by the model. -const ALLOWED_TOP_LEVEL_ANNOTATIONS: &[&str] = &["$schema", "_comment"]; + if let Some(builtin) = builtin_test_server { + if !builtin { + return Err(WxcError::ConfigParse( + "network.proxy.builtinTestServer must be true when present".to_string(), + )); + } + if localhost.is_some() || url.is_some() { + return Err(WxcError::ConfigParse( + "When builtinTestServer is true, no other proxy options may be set".to_string(), + )); + } + return Ok(ProxyConfig { + address: Some(proxy_addr), + builtin_test_server: true, + }); + } -/// Rejects unrecognised top-level fields captured by a `#[serde(flatten)]` map. -/// -/// This is the structural half of schema enforcement at the trust boundary: -/// typos and silently-dropped fields (for example `fileSystem` instead of -/// `filesystem`, or a stray `policy` block) now fail loudly with a precise -/// error instead of being ignored, which previously meant the associated -/// policy was never applied. -fn reject_unknown_top_level_fields( - unknown: &HashMap, - logger: &mut Logger, -) -> Result<(), WxcError> { - let mut keys: Vec<&str> = unknown - .keys() - .map(String::as_str) - .filter(|k| !ALLOWED_TOP_LEVEL_ANNOTATIONS.contains(k)) - .collect(); - if keys.is_empty() { - return Ok(()); + if let Some(port) = localhost { + if port == 0 { + return Err(WxcError::ConfigParse( + "network.proxy.localhost must be a port between 1 and 65535".to_string(), + )); + } + proxy_addr.port = port; + return Ok(ProxyConfig { + address: Some(proxy_addr), + builtin_test_server: false, + }); } - keys.sort_unstable(); - let msg = format!( - "Unknown top-level field(s) in config: {}. Remove them or check for typos \ - (field names are case-sensitive, e.g. 'filesystem' not 'fileSystem').", - keys.join(", ") - ); - logger.log_line(&msg); - Err(WxcError::ConfigParse(msg)) + + if let Some(url_str) = url { + let parsed = url::Url::parse(&url_str) + .map_err(|e| WxcError::ConfigParse(format!("network.proxy.url is invalid: {e}")))?; + + let host = parsed + .host_str() + .ok_or_else(|| { + WxcError::ConfigParse(format!( + "network.proxy.url must include a host (e.g., http://localhost:8080), got: {url_str}" + )) + })? + .to_string(); + let port = parsed.port().ok_or_else(|| { + WxcError::ConfigParse(format!( + "network.proxy.url must include a port (e.g., http://localhost:8080), got: {url_str}" + )) + })?; + + return Ok(ProxyConfig { + address: Some(ProxyAddress::from_url(&url_str, host, port)), + builtin_test_server: false, + }); + } + + Err(WxcError::ConfigParse( + "network.proxy must specify builtinTestServer, localhost, or url".to_string(), + )) } -fn present_backend_sections(raw: &RawConfig) -> Vec<&'static str> { +fn present_backend_sections(cfg: &wire::MxcConfig) -> Vec<&'static str> { let mut sections: Vec<&'static str> = Vec::new(); let mut push = |backend: ContainmentBackend| { if let Some(path) = backend.section_path() { sections.push(path); } }; - if raw.process_container.is_some() { + if cfg.process_container.is_some() { push(ContainmentBackend::ProcessContainer); } - if raw.lxc.is_some() { + if cfg.lxc.is_some() { push(ContainmentBackend::Lxc); } - if raw.seatbelt.is_some() { + if cfg.seatbelt.is_some() { push(ContainmentBackend::Seatbelt); } - if let Some(experimental) = raw.experimental.as_ref() { + if let Some(experimental) = cfg.experimental.as_ref() { if experimental.windows_sandbox.is_some() { push(ContainmentBackend::WindowsSandbox); } @@ -778,54 +485,165 @@ fn validate_experimental_backend_keys( Err(WxcError::ConfigParse(msg)) } -// `require_process = false` allows state-aware non-exec phases to omit the -// `process` block entirely; those phases leave `script_code` / `working_directory` -// / `script_timeout` / `env` at their defaults and never read them. -// -/// Convert a raw seatbelt JSON block into the validated domain struct. -fn make_seatbelt_config(raw_sb: RawSeatbelt) -> SeatbeltConfig { +/// Convert a typed `wire::Seatbelt` block into the validated domain struct. +fn make_seatbelt_config(sb: wire::Seatbelt) -> SeatbeltConfig { + // Destructure (no `..`) so adding a wire field without mapping it is a + // compile error rather than a silent runtime drop. + let wire::Seatbelt { + profile_override, + gui_access, + launch_method, + nested_pty, + keychain_access, + extra_mach_lookups, + } = sb; SeatbeltConfig { - profile_override: raw_sb.profile_override, - gui_access: raw_sb.gui_access.unwrap_or(false), - launch_method: raw_sb.launch_method.unwrap_or_default(), - nested_pty: raw_sb.nested_pty.unwrap_or(true), - keychain_access: raw_sb.keychain_access.unwrap_or(false), - extra_mach_lookups: raw_sb.extra_mach_lookups.unwrap_or_default(), + profile_override, + gui_access: gui_access.unwrap_or(false), + launch_method: launch_method + .map(map_wire_launch_method) + .unwrap_or_default(), + nested_pty: nested_pty.unwrap_or(true), + keychain_access: keychain_access.unwrap_or(false), + extra_mach_lookups: extra_mach_lookups.unwrap_or_default(), + } +} + +/// Map the wire Seatbelt launch method to the domain enum. +fn map_wire_launch_method(m: wire::LaunchMethod) -> crate::models::LaunchMethod { + match m { + wire::LaunchMethod::Exec => crate::models::LaunchMethod::Exec, + wire::LaunchMethod::Open => crate::models::LaunchMethod::Open, + } +} + +/// Lowercase wire-format string for a UI isolation level (matches the schema +/// enum values consumed downstream). +fn wire_ui_isolation_str(iso: &wire::UiIsolation) -> &'static str { + match iso { + wire::UiIsolation::Desktop => "desktop", + wire::UiIsolation::Handles => "handles", + wire::UiIsolation::Atoms => "atoms", + wire::UiIsolation::Container => "container", + } +} + +/// Map the wire lifecycle phase to the domain `Phase`. +fn map_wire_phase(p: wire::Phase) -> Phase { + match p { + wire::Phase::Provision => Phase::Provision, + wire::Phase::Start => Phase::Start, + wire::Phase::Exec => Phase::Exec, + wire::Phase::Stop => Phase::Stop, + wire::Phase::Deprovision => Phase::Deprovision, } } -// `allow_missing_command` further relaxes the `require_process == true` arms -// so that a CLI command-line override (provided by the driver after parsing) -// can stand in for `process.commandLine`. When set, a missing or empty -// `commandLine` is silently accepted and `script_code` is left empty. -fn convert_raw_config_inner( - raw: RawConfig, +/// Map the wire IsolationSession sizing profile to the domain enum. +fn map_wire_isolation_configuration_id( + id: wire::IsolationConfigurationId, +) -> IsolationSessionConfigurationId { + match id { + wire::IsolationConfigurationId::Small => IsolationSessionConfigurationId::Small, + wire::IsolationConfigurationId::Medium => IsolationSessionConfigurationId::Medium, + wire::IsolationConfigurationId::Large => IsolationSessionConfigurationId::Large, + wire::IsolationConfigurationId::Composable => IsolationSessionConfigurationId::Composable, + } +} + +/// Map the wire Entra user bundle to the domain struct. +fn map_wire_isolation_user(u: wire::IsolationUser) -> IsolationSessionUser { + // Destructure (no `..`) so a new wire field fails to compile until mapped. + let wire::IsolationUser { upn, wam_token } = u; + IsolationSessionUser { upn, wam_token } +} + +/// Resolve the `containment` wire enum to a concrete domain backend. +/// +/// `None` (omitted) and the abstract intent `Process` resolve to the OS-native +/// process sandbox per target_os; `Vm` resolves to the host's VM-class backend. +/// Deprecated spellings (`appcontainer`, `macos_sandbox`) are accepted via +/// `#[serde(alias)]` on the wire enum and arrive here already mapped to the +/// canonical variant. Concrete backends map verbatim. +fn map_wire_containment(c: Option<&wire::Containment>) -> ContainmentBackend { + use wire::Containment as W; + match c { + None | Some(W::Process) => { + #[cfg(target_os = "linux")] + { + ContainmentBackend::Bubblewrap + } + #[cfg(target_os = "macos")] + { + ContainmentBackend::Seatbelt + } + #[cfg(not(any(target_os = "linux", target_os = "macos")))] + { + ContainmentBackend::ProcessContainer + } + } + Some(W::ProcessContainer) => ContainmentBackend::ProcessContainer, + Some(W::Vm) => { + #[cfg(target_os = "windows")] + { + ContainmentBackend::WindowsSandbox + } + #[cfg(not(target_os = "windows"))] + { + ContainmentBackend::Vm + } + } + Some(W::WindowsSandbox) => ContainmentBackend::WindowsSandbox, + Some(W::Lxc) => ContainmentBackend::Lxc, + Some(W::Microvm) => ContainmentBackend::MicroVm, + Some(W::Hyperlight) => ContainmentBackend::Hyperlight, + Some(W::Wslc) => ContainmentBackend::Wslc, + Some(W::Seatbelt) => ContainmentBackend::Seatbelt, + Some(W::IsolationSession) => ContainmentBackend::IsolationSession, + Some(W::Bubblewrap) => ContainmentBackend::Bubblewrap, + } +} + +// `allow_missing_command` relaxes the `require_process == true` arms so that a +// CLI command-line override (provided by the driver after parsing) can stand in +// for `process.commandLine`. When set, a missing or empty `commandLine` is +// silently accepted and `script_code` is left empty. +fn convert_wire_config( + cfg: wire::MxcConfig, logger: &mut Logger, require_process: bool, allow_missing_command: bool, ) -> Result { - // Reject unrecognised top-level fields before anything else, so typos and - // stray sections fail loudly instead of being silently ignored. - reject_unknown_top_level_fields(&raw.unknown, logger)?; + // `phase` / `sandboxId` are state-aware-only fields. The state-aware path + // consumes them before delegating here, so if either is still present the + // input is a state-aware-shaped payload sent to a one-shot entry point; + // reject it loudly rather than silently executing it as a one-shot. + if cfg.phase.is_some() { + let msg = "'phase' is only valid on state-aware lifecycle requests".to_string(); + logger.log_line(&msg); + return Err(WxcError::ConfigParse(msg)); + } + if cfg.sandbox_id.is_some() { + let msg = "'sandboxId' is only valid on state-aware lifecycle requests".to_string(); + logger.log_line(&msg); + return Err(WxcError::ConfigParse(msg)); + } - // Captured before `raw` fields are moved out below. - let present_backend_sections = present_backend_sections(&raw); + // Backend sections present in the config (captured before fields move out). + let present_backend_sections = present_backend_sections(&cfg); - // New top-level fields - let schema_version = raw.version.unwrap_or_default(); + let schema_version = cfg.version.unwrap_or_default(); - // Validate the schema version up front so an unsupported version fails fast, - // before any of the per-section conversion work below. + // Validate the schema version up front so an unsupported version fails fast. validate_schema_version(&schema_version, logger)?; - let container_id = raw.container_id.unwrap_or_default(); + let container_id = cfg.container_id.unwrap_or_default(); - // Process section: required for one-shot and for state-aware exec; absent - // is allowed for state-aware non-exec phases (require_process == false) - // or when the driver has signalled a CLI command-line override is - // present via `allow_missing_command`. + // Process section: required for one-shot and state-aware exec; optional for + // non-exec state-aware phases (require_process == false) or when the driver + // signalled a CLI command-line override (allow_missing_command). let command_required = require_process && !allow_missing_command; - let (script_code, working_directory, script_timeout, env) = match raw.process { + let (script_code, working_directory, script_timeout, env) = match cfg.process { Some(process) => { let script_code = match process.command_line { Some(s) if !s.is_empty() => s, @@ -844,8 +662,7 @@ fn convert_raw_config_inner( _ => String::new(), }; - // Null bytes can be used to hide malicious payloads from audit logs or - // other inspection. + // Null bytes can hide malicious payloads from audit logs. if script_code.contains('\0') { return Err(WxcError::ConfigParse( "process.commandLine must not contain null bytes".to_string(), @@ -867,135 +684,34 @@ fn convert_raw_config_inner( None => (String::new(), String::new(), 0, Vec::new()), }; - // Containment backend selection. - // - // The `containment` wire field is dual-purpose: callers may pass either - // (a) an abstract intent (e.g. "process") that names a *kind* of - // isolation and lets the binary pick the host-appropriate runner, - // or - // (b) a concrete backend id (e.g. "processcontainer", "lxc", "seatbelt") - // that pins the runner explicitly. - // - // Both forms target the same internal `ContainmentBackend` enum. The - // match below recognises every concrete id verbatim and resolves the - // abstract intents into the appropriate concrete variant per - // target_os. Any future concrete-only backend just needs an arm; any - // future abstract intent should resolve here (and likewise in - // `parse_containment_str` below for the state-aware path). - // - // Default resolution (omitted `containment`) and the abstract intent - // `"process"` map to the OS-native process sandbox on each platform: - // * Windows -> ProcessContainer (AppContainer) - // * macOS -> Seatbelt - // * Linux -> Bubblewrap (lightweight, unprivileged process sandbox) - // LXC is treated as a full Linux container and is only selected when - // explicitly requested via `"lxc"`; `"processcontainer"` continues to - // route to ProcessContainer (which `lxc-exec` falls back to LXC for). - let containment = match raw.containment.as_deref() { - None => { - #[cfg(target_os = "linux")] - { - ContainmentBackend::Bubblewrap - } - #[cfg(target_os = "macos")] - { - ContainmentBackend::Seatbelt - } - #[cfg(not(any(target_os = "linux", target_os = "macos")))] - { - ContainmentBackend::ProcessContainer - } - } - Some("processcontainer") => ContainmentBackend::ProcessContainer, - Some("appcontainer") => { - logger.log_line( - "[deprecated] containment value 'appcontainer' is a legacy alias for 'processcontainer'; \ - update your config to use 'processcontainer' (the 'appcontainer' alias may be removed in a future schema version).", - ); - ContainmentBackend::ProcessContainer - } - Some("process") => { - // Abstract intent: the caller wants the OS-native process - // sandbox. Resolves to ProcessContainer on Windows, Bubblewrap - // on Linux, and Seatbelt on macOS. Callers who want LXC (a - // full container) must request it explicitly via "lxc". - #[cfg(target_os = "linux")] - { - ContainmentBackend::Bubblewrap - } - #[cfg(target_os = "macos")] - { - ContainmentBackend::Seatbelt - } - #[cfg(not(any(target_os = "linux", target_os = "macos")))] - { - ContainmentBackend::ProcessContainer - } - } - Some("windows_sandbox") => ContainmentBackend::WindowsSandbox, - Some("wslc") => ContainmentBackend::Wslc, - Some("lxc") => ContainmentBackend::Lxc, - Some("vm") => { - // Abstract intent: full hardware-virtualised VM isolation. - // Today the only concrete VM backend is Windows Sandbox; on - // non-Windows targets we fall through to the historical - // `Vm` variant which the host binaries surface as an - // explicit "not implemented" error. - #[cfg(target_os = "windows")] - { - ContainmentBackend::WindowsSandbox - } - #[cfg(not(target_os = "windows"))] - { - ContainmentBackend::Vm - } - } - Some("microvm") => ContainmentBackend::MicroVm, - Some("isolation_session") => ContainmentBackend::IsolationSession, - Some("seatbelt") => ContainmentBackend::Seatbelt, - Some("macos_sandbox") => { - logger.log_line( - "[deprecated] containment value 'macos_sandbox' is a legacy alias for 'seatbelt'; \ - update your config to use 'seatbelt' (the 'macos_sandbox' alias may be removed in a future schema version).", - ); - ContainmentBackend::Seatbelt - } - Some("hyperlight") => ContainmentBackend::Hyperlight, - Some("bubblewrap") => ContainmentBackend::Bubblewrap, - Some(other) => { - let msg = format!( - "Invalid containment value '{}' (must be 'process', 'processcontainer', 'windows_sandbox', 'isolation_session', 'wslc', 'lxc', 'vm', 'microvm', 'seatbelt', 'hyperlight', or 'bubblewrap')", - other - ); - logger.log_line(&msg); - return Err(WxcError::ConfigParse(msg)); - } - }; + // Containment backend selection. The wire enum has already constrained the + // value to a known variant (invalid strings fail at deserialize); abstract + // intents and the omitted case resolve to the OS-native backend here. + let containment = map_wire_containment(cfg.containment.as_ref()); validate_single_backend_section(containment.clone(), &present_backend_sections, logger)?; // LXC configuration - let lxc_config = { - let raw_lxc = raw.lxc.unwrap_or_default(); - LxcConfig { - distribution: raw_lxc.distribution.unwrap_or_default(), - release: raw_lxc.release.unwrap_or_default(), - } + let lxc_config = match cfg.lxc { + Some(l) => LxcConfig { + distribution: l.distribution.unwrap_or_default(), + release: l.release.unwrap_or_default(), + }, + None => LxcConfig::default(), }; let mut policy = ContainerPolicy::default(); // ProcessContainer section. Holds settings that apply to the Windows - // process-level backend regardless of whether the runner picks the - // legacy AppContainer implementation (which honors `capabilities`, - // `learningMode`, `leastPrivilege`) or the newer BaseContainer - // implementation (which honors `ui`). - if let Some(ac) = raw.process_container { + // process-level backend regardless of whether the runner picks the legacy + // AppContainer implementation (capabilities/learningMode/leastPrivilege) or + // the newer BaseContainer implementation (ui). + if let Some(ac) = cfg.process_container { if let Some(lp) = ac.least_privilege { policy.least_privilege_mode = lp; } - // learningMode handling differs between debug and release + // learningMode handling differs between debug and release. if ac.learning_mode.unwrap_or(false) { #[cfg(debug_assertions)] { @@ -1010,12 +726,11 @@ fn convert_raw_config_inner( } } - // Add explicit capabilities if let Some(caps) = ac.capabilities { policy.capabilities.extend(caps); } - // SECURITY: Strip permissiveLearningMode in release builds + // SECURITY: Strip permissiveLearningMode in release builds. #[cfg(not(debug_assertions))] { policy.capabilities.retain(|cap| { @@ -1028,10 +743,14 @@ fn convert_raw_config_inner( }); } - // BaseProcessContainer-specific UI config + // BaseProcessContainer-specific UI config. if let Some(raw_ui) = ac.ui { - policy.base_process_ui.isolation = - raw_ui.isolation.unwrap_or_else(|| "container".to_string()); + policy.base_process_ui.isolation = raw_ui + .isolation + .as_ref() + .map(wire_ui_isolation_str) + .unwrap_or("container") + .to_string(); policy.base_process_ui.desktop_system_control = raw_ui.desktop_system_control.unwrap_or(false); policy.base_process_ui.system_settings = @@ -1041,7 +760,7 @@ fn convert_raw_config_inner( } // Filesystem section - if let Some(fscfg) = raw.filesystem { + if let Some(fscfg) = cfg.filesystem { if let Some(v) = fscfg.denied_paths { policy.denied_paths = v; } @@ -1055,16 +774,16 @@ fn convert_raw_config_inner( validate_filesystem_paths(&policy, logger)?; // Fallback section - if let Some(fbcfg) = raw.fallback { + if let Some(fbcfg) = cfg.fallback { if let Some(v) = fbcfg.allow_dacl_mutation { policy.fallback.allow_dacl_mutation = v; } } // Network section - if let Some(net) = raw.network { - if let Some(proxy_value) = net.proxy { - let proxy_config = parse_proxy_config(&proxy_value)?; + if let Some(net) = cfg.network { + if let Some(proxy) = net.proxy { + let proxy_config = convert_wire_proxy(proxy)?; if proxy_config.is_enabled() && containment != ContainmentBackend::ProcessContainer && containment != ContainmentBackend::Bubblewrap @@ -1078,33 +797,17 @@ fn convert_raw_config_inner( } if let Some(p) = net.default_policy { - policy.default_network_policy = match p.as_str() { - "allow" => NetworkPolicy::Allow, - "block" => NetworkPolicy::Block, - other => { - let msg = format!( - "Invalid network.defaultPolicy value '{}' (must be 'allow' or 'block')", - other - ); - logger.log_line(&msg); - return Err(WxcError::ConfigParse(msg)); - } + policy.default_network_policy = match p { + wire::NetworkPolicy::Allow => NetworkPolicy::Allow, + wire::NetworkPolicy::Block => NetworkPolicy::Block, }; } if let Some(m) = net.enforcement_mode { - policy.network_enforcement_mode = match m.as_str() { - "capabilities" => NetworkEnforcementMode::Capabilities, - "firewall" => NetworkEnforcementMode::Firewall, - "both" => NetworkEnforcementMode::Both, - other => { - let msg = format!( - "Invalid network.enforcementMode value '{}' (must be 'capabilities', 'firewall', or 'both')", - other - ); - logger.log_line(&msg); - return Err(WxcError::ConfigParse(msg)); - } + policy.network_enforcement_mode = match m { + wire::NetworkEnforcement::Capabilities => NetworkEnforcementMode::Capabilities, + wire::NetworkEnforcement::Firewall => NetworkEnforcementMode::Firewall, + wire::NetworkEnforcement::Both => NetworkEnforcementMode::Both, }; } @@ -1120,9 +823,8 @@ fn convert_raw_config_inner( } // Bubblewrap is unprivileged by design; iptables-based enforcement - // (firewall / both) requires CAP_NET_ADMIN, which defeats the - // backend's privilege story. Reject the combination explicitly so - // users get a clear error instead of an opaque runtime failure. + // (firewall / both) requires CAP_NET_ADMIN, which defeats the backend's + // privilege story. Reject the combination explicitly. if containment == ContainmentBackend::Bubblewrap && policy.network_proxy.is_enabled() && matches!( @@ -1138,13 +840,10 @@ fn convert_raw_config_inner( return Err(WxcError::ConfigParse(msg.to_string())); } - // External proxy (`network.proxy.url` / `network.proxy.localhost`) - // enforces its own policy — the runner does NOT forward - // `allowedHosts` / `blockedHosts` / `defaultPolicy` to it. Reject - // configs that combine an external proxy with host lists or a - // restrictive default, otherwise users get a silently weaker - // enforcement than a no-proxy `defaultPolicy: "block"` config - // would have produced. + // External proxy (`url` / `localhost`) enforces its own policy — the + // runner does NOT forward host lists to it. Reject configs that combine + // an external proxy with host lists or a restrictive default, otherwise + // users get silently weaker enforcement. if containment == ContainmentBackend::Bubblewrap && policy.network_proxy.is_enabled() && !policy.network_proxy.builtin_test_server @@ -1162,14 +861,9 @@ fn convert_raw_config_inner( return Err(WxcError::ConfigParse(msg.to_string())); } - // Cooperative-model warning: when the builtin test proxy is paired - // with `defaultPolicy: "block"` and no allowlist, well-behaved - // HTTP clients are denied at the proxy, but raw-socket clients - // (anything that ignores HTTP_PROXY/HTTPS_PROXY) still reach the - // host network because the sandbox shares the host netns in proxy - // mode. Surface this at config-validation time so users porting a - // working hard-block config don't silently lose enforcement when - // they add a proxy block. + // Cooperative-model warning: builtin test proxy + defaultPolicy 'block' + // with no allowlist denies well-behaved HTTP clients at the proxy, but + // raw-socket clients still reach the host network. if containment == ContainmentBackend::Bubblewrap && policy.network_proxy.is_enabled() && policy.default_network_policy == NetworkPolicy::Block @@ -1188,19 +882,19 @@ fn convert_raw_config_inner( } // Lifecycle section - let lifecycle = { - let lc = raw.lifecycle.unwrap_or_default(); - let destroy_on_exit = lc.destroy_on_exit.unwrap_or(true); - let preserve_policy = lc.preserve_policy.unwrap_or(false); - - LifecycleConfig { - destroy_on_exit, - preserve_policy, - } + let lifecycle = match cfg.lifecycle { + Some(lc) => LifecycleConfig { + destroy_on_exit: lc.destroy_on_exit.unwrap_or(true), + preserve_policy: lc.preserve_policy.unwrap_or(false), + }, + None => LifecycleConfig { + destroy_on_exit: true, + preserve_policy: false, + }, }; - // Experimental section (parsed but only applied when --experimental flag is set) - let experimental = if let Some(raw_exp) = raw.experimental { + // Experimental section (parsed but only applied when --experimental is set). + let experimental = if let Some(raw_exp) = cfg.experimental { let test = raw_exp.test.map(|t| TestFeatureConfig::from_raw(t.message)); let windows_sandbox = raw_exp.windows_sandbox.map(|sb| { let mut config = WindowsSandboxConfig::default(); @@ -1244,36 +938,12 @@ fn convert_raw_config_inner( logger.log_line(&msg); return Err(WxcError::ConfigParse(msg)); } - let protocol = m - .protocol - .unwrap_or_else(|| "tcp".to_string()) - .to_lowercase(); - if protocol != "tcp" && protocol != "udp" { - let msg = format!( - "experimental.wslc.portMappings[{idx}]: protocol '{protocol}' \ - not supported; expected 'tcp'" - ); - logger.log_line(&msg); - return Err(WxcError::ConfigParse(msg)); - } - // The WSLC SDK header (Microsoft.WSL.Containers 2.8.1, - // vendored under external/wslc-sdk/) declares - // WSLC_PORT_PROTOCOL_UDP = 1, but the shipped runtime - // returns E_NOTIMPL (0x80004001) when UDP is actually - // passed to WslcSetContainerSettingsPortMappings. Reject - // UDP at parse time with a clear message until a future - // SDK version implements it. - if protocol == "udp" { - let msg = format!( - "experimental.wslc.portMappings[{idx}]: protocol 'udp' is \ - not supported by the vendored WSLC SDK runtime \ - (Microsoft.WSL.Containers 2.8.1). Only 'tcp' is currently \ - implemented. UDP support will be enabled when a future SDK \ - version ships it." - ); - logger.log_line(&msg); - return Err(WxcError::ConfigParse(msg)); - } + // Only TCP is representable in the wire model + // (TransportProtocol is tcp-only); a `udp` value is rejected + // at deserialize. The vendored WSLC SDK runtime + // (Microsoft.WSL.Containers 2.8.1) returns E_NOTIMPL for UDP, + // so only TCP is currently supported. + let protocol = "tcp".to_string(); converted.push(PortMapping { windows_port: m.windows_port, container_port: m.container_port, @@ -1282,8 +952,9 @@ fn convert_raw_config_inner( } // Reject duplicate (windowsPort, protocol) entries. Same host // port on TCP+UDP would in principle be legal, but UDP is - // rejected earlier; the second protocol dimension is retained - // in the dedupe key in case UDP support is enabled later. + // rejected at deserialize (the wire model is tcp-only); the + // second protocol dimension is retained in the dedupe key in + // case UDP support is enabled later. let mut seen: std::collections::HashSet<(u16, &str)> = std::collections::HashSet::new(); for pm in &converted { @@ -1306,26 +977,12 @@ fn convert_raw_config_inner( let isolation_session = raw_exp.isolation_session.map(|as_cfg| { let mut config = IsolationSessionConfig::default(); if let Some(id) = as_cfg.configuration_id { - use crate::models::IsolationSessionConfigurationId; - config.configuration_id = match id.as_str() { - "small" => IsolationSessionConfigurationId::Small, - "medium" => IsolationSessionConfigurationId::Medium, - "large" => IsolationSessionConfigurationId::Large, - "composable" => IsolationSessionConfigurationId::Composable, - _ => { - logger.log_line(&format!( - "Unknown isolation_session configurationId '{}', defaulting to 'composable'", - id - )); - IsolationSessionConfigurationId::Composable - } - }; + config.configuration_id = map_wire_isolation_configuration_id(id); } - config.user = as_cfg.user; + config.user = as_cfg.user.map(map_wire_isolation_user); config }); - let seatbelt = raw_exp.seatbelt; - if seatbelt.is_some() { + if raw_exp.seatbelt.is_some() { let msg = "'experimental.seatbelt' has moved to the stable section; \ use top-level 'seatbelt' instead." .to_string(); @@ -1344,14 +1001,14 @@ fn convert_raw_config_inner( // Top-level `seatbelt` config. Configs using `experimental.seatbelt` are // rejected above. - let seatbelt = raw.seatbelt.map(make_seatbelt_config); + let seatbelt = cfg.seatbelt.map(make_seatbelt_config); // UI section - if let Some(raw_ui) = raw.ui { - let clipboard = match raw_ui.clipboard.as_deref() { - Some("read") => ClipboardPolicy::Read, - Some("write") => ClipboardPolicy::Write, - Some("all") => ClipboardPolicy::All, + if let Some(raw_ui) = cfg.ui { + let clipboard = match raw_ui.clipboard { + Some(wire::ClipboardPolicy::Read) => ClipboardPolicy::Read, + Some(wire::ClipboardPolicy::Write) => ClipboardPolicy::Write, + Some(wire::ClipboardPolicy::All) => ClipboardPolicy::All, _ => ClipboardPolicy::None, }; policy.ui = UiPolicy { @@ -1380,134 +1037,112 @@ fn convert_raw_config_inner( }) } -fn convert_raw_state_aware( - raw: RawStateAwareRequest, +fn convert_wire_state_aware( + mut value: serde_json::Value, logger: &mut Logger, allow_missing_command: bool, ) -> Result { - // Reject unrecognised top-level fields on the state-aware envelope before - // building the one-shot surrogate (whose `unknown` map is always empty). - reject_unknown_top_level_fields(&raw.unknown, logger)?; + // Capture the raw `experimental` block before typed deserialize; it is typed + // per-backend at dispatch time, not here. + let experimental_raw = value + .as_object_mut() + .and_then(|map| map.remove("experimental")); - let phase = Phase::from_wire(&raw.phase).map_err(|e| { - let msg = e.message.clone(); - logger.log_line(&msg); - WxcError::ConfigParse(msg) + let mut cfg: wire::MxcConfig = serde_json::from_value(value).map_err(|e| { + logger.log_line("Error parsing JSON"); + WxcError::ConfigParse(format!("JSON parse error: {}", e)) })?; - let containment = match raw.containment.as_deref() { - None => None, - Some(wire) => Some(parse_containment_str(wire, logger)?), + // `phase` is the state-aware discriminator and is constrained by the wire + // enum; absence here would be a logic error in the caller's discrimination. + let phase = match cfg.phase.take() { + Some(p) => map_wire_phase(p), + None => { + return Err(WxcError::ConfigParse( + "Missing required field: phase".to_string(), + )); + } }; - validate_experimental_backend_keys(containment.as_ref(), raw.experimental.as_ref(), logger)?; - - // Build a RawConfig surrogate so the inner ExecutionRequest is populated by the - // same conversion path one-shot uses for cross-cutting wire fields. - let surrogate = RawConfig { - version: raw.version, - container_id: raw.container_id, - process: raw.process, - lifecycle: None, - containment: raw.containment, - process_container: None, - lxc: None, - filesystem: raw.filesystem, - fallback: raw.fallback, - network: raw.network, - ui: raw.ui, - // The state-aware experimental block has a different shape from the - // one-shot RawExperimental; it is preserved separately on - // ParsedStateAwareRequest as raw JSON. - experimental: None, - seatbelt: None, - unknown: HashMap::new(), - }; + // Resolved backend, present only when the request carried `containment`. + let containment = cfg + .containment + .as_ref() + .map(|c| map_wire_containment(Some(c))); + + // Mirror the one-shot rejection of moved-to-stable experimental sections. + // The one-shot path errors on `experimental.seatbelt` in `convert_wire_config`, + // but the state-aware path peels `experimental` into `experimental_raw` + // before that runs, so without this check the block would be silently + // discarded (the same silent-policy-drop class as the F1 stable sections). + if let Some(serde_json::Value::Object(exp)) = experimental_raw.as_ref() { + for key in ["seatbelt", "macos_sandbox"] { + if exp.contains_key(key) { + let msg = format!( + "'experimental.{key}' has moved to the stable section; \ + use top-level 'seatbelt' instead." + ); + logger.log_line(&msg); + return Err(WxcError::ConfigParse(msg)); + } + } + } + + validate_experimental_backend_keys(containment.as_ref(), experimental_raw.as_ref(), logger)?; + + let sandbox_id = cfg.sandbox_id.clone(); + + // State-aware requests carry only cross-cutting fields (process / + // filesystem / network / ui) plus the experimental backend block. One-shot- + // only stable sections and lifecycle are not valid here; reject them + // explicitly rather than silently discarding a policy the caller believes + // is in effect. + let mut stray: Vec<&'static str> = Vec::new(); + if cfg.seatbelt.is_some() { + stray.push("seatbelt"); + } + if cfg.process_container.is_some() { + stray.push("processContainer"); + } + if cfg.lxc.is_some() { + stray.push("lxc"); + } + if cfg.lifecycle.is_some() { + stray.push("lifecycle"); + } + if !stray.is_empty() { + let msg = format!( + "State-aware lifecycle requests do not accept one-shot section(s): {}. \ + Remove them; per-backend policy and lifecycle are fixed at provision time.", + stray.join(", ") + ); + logger.log_line(&msg); + return Err(WxcError::ConfigParse(msg)); + } + + // Populate the inner ExecutionRequest from cross-cutting fields only. Clear + // the state-aware-only fields (already consumed above) and the + // now-validated-absent stable sections so the shared one-shot converter + // sees a clean surrogate and its `phase`/`sandboxId` guard passes. + cfg.sandbox_id = None; + cfg.experimental = None; + cfg.seatbelt = None; + cfg.process_container = None; + cfg.lxc = None; + cfg.lifecycle = None; let require_process = phase == Phase::Exec; - let request = - convert_raw_config_inner(surrogate, logger, require_process, allow_missing_command)?; + let request = convert_wire_config(cfg, logger, require_process, allow_missing_command)?; Ok(ParsedStateAwareRequest { request, phase, containment, - sandbox_id: raw.sandbox_id, - experimental_raw: raw.experimental, + sandbox_id, + experimental_raw, }) } -/// State-aware path: parse the `containment` wire field on a per-phase -/// request envelope. -/// -/// Mirrors the dual-acceptance contract of the one-shot match in -/// `convert_raw_config_inner`: the input may be either an abstract intent -/// (resolved per `target_os`) or a concrete backend id. Keep the two -/// match expressions in lockstep when adding new values. -fn parse_containment_str(s: &str, logger: &mut Logger) -> Result { - match s { - "processcontainer" => Ok(ContainmentBackend::ProcessContainer), - "appcontainer" => { - logger.log_line( - "[deprecated] containment value 'appcontainer' is a legacy alias for 'processcontainer'; \ - update your config to use 'processcontainer' (the 'appcontainer' alias may be removed in a future schema version).", - ); - Ok(ContainmentBackend::ProcessContainer) - } - "process" => { - // Abstract intent: the caller wants the OS-native process - // sandbox. Resolves to ProcessContainer on Windows, Bubblewrap - // on Linux, and Seatbelt on macOS. Callers who want LXC (a - // full container) must request it explicitly via "lxc". - #[cfg(target_os = "linux")] - { - Ok(ContainmentBackend::Bubblewrap) - } - #[cfg(target_os = "macos")] - { - Ok(ContainmentBackend::Seatbelt) - } - #[cfg(not(any(target_os = "linux", target_os = "macos")))] - { - Ok(ContainmentBackend::ProcessContainer) - } - } - "windows_sandbox" => Ok(ContainmentBackend::WindowsSandbox), - "wslc" => Ok(ContainmentBackend::Wslc), - "lxc" => Ok(ContainmentBackend::Lxc), - "vm" => { - #[cfg(target_os = "windows")] - { - Ok(ContainmentBackend::WindowsSandbox) - } - #[cfg(not(target_os = "windows"))] - { - Ok(ContainmentBackend::Vm) - } - } - "microvm" => Ok(ContainmentBackend::MicroVm), - "isolation_session" => Ok(ContainmentBackend::IsolationSession), - "seatbelt" => Ok(ContainmentBackend::Seatbelt), - "hyperlight" => Ok(ContainmentBackend::Hyperlight), - "bubblewrap" => Ok(ContainmentBackend::Bubblewrap), - "macos_sandbox" => { - logger.log_line( - "[deprecated] containment value 'macos_sandbox' is a legacy alias for 'seatbelt'; \ - update your config to use 'seatbelt' (the 'macos_sandbox' alias may be removed in a future schema version).", - ); - Ok(ContainmentBackend::Seatbelt) - } - other => { - let msg = format!( - "Invalid containment value '{}' (must be 'process', 'processcontainer', 'windows_sandbox', 'isolation_session', 'wslc', 'lxc', 'vm', 'microvm', 'seatbelt', 'hyperlight', or 'bubblewrap')", - other - ); - logger.log_line(&msg); - Err(WxcError::ConfigParse(msg)) - } - } -} - #[cfg(test)] mod tests { use super::*; @@ -1644,7 +1279,16 @@ mod tests { MxcRequest::StateAware(p) => { assert_eq!(p.phase, Phase::Start); assert_eq!(p.sandbox_id.as_deref(), Some("iso:abcd1234")); - assert!(p.experimental_raw.is_some()); + // Assert the nested experimental payload survives extraction + // unchanged (not merely that the block is present), since the + // dispatcher types it per-backend from this raw value. + let exp = p.experimental_raw.expect("experimental block present"); + assert_eq!( + exp, + serde_json::json!({ + "isolation_session": {"start": {"configurationId": "small"}} + }) + ); } MxcRequest::OneShot(_) => panic!("expected state-aware"), } @@ -1812,8 +1456,12 @@ mod tests { let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); - let result = load_request(&encoded, &mut logger, true); - assert!(result.is_err()); + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("unknown variant") && msg.contains("invalid"), + "expected serde unknown-variant rejection, got: {msg}" + ); } #[test] @@ -1823,8 +1471,12 @@ mod tests { let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); - let result = load_request(&encoded, &mut logger, true); - assert!(result.is_err()); + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("unknown variant") && msg.contains("invalid"), + "expected serde unknown-variant rejection, got: {msg}" + ); } #[test] @@ -2251,9 +1903,8 @@ mod tests { #[test] fn hyperlight_containment_value_parses() { - // Lock in that `"hyperlight"` is accepted by the parser (mirrors - // the `convert_raw_config_inner` arm and keeps `parse_containment_str` - // in sync for the state-aware path). + // Lock in that `"hyperlight"` is accepted by the parser (the + // `map_wire_containment` arm handles both one-shot and state-aware). let json = r#"{"process": {"commandLine": "echo hello"}, "containment": "hyperlight"}"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); @@ -2297,8 +1948,12 @@ mod tests { let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); - let result = load_request(&encoded, &mut logger, true); - assert!(result.is_err()); + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("unknown variant") && msg.contains("docker"), + "expected serde unknown-variant rejection, got: {msg}" + ); } #[test] @@ -2881,6 +2536,129 @@ mod tests { assert!(result.is_err(), "fileSystem typo should be rejected"); } + #[test] + fn nested_unknown_field_rejected() { + // The stable surface is closed at every level (deny_unknown_fields): + // an unknown *nested* field must be rejected, not just top-level ones. + let json = r#"{"process": {"commandLine": "echo hi", "bogus": 1}}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("unknown field") && msg.contains("bogus"), + "nested unknown field should be rejected, got: {msg}" + ); + } + + #[test] + fn nested_proxy_unknown_field_rejected() { + let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "processcontainer", "network": {"proxy": {"localhost": 8080, "unexpected": true}}}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("unknown field") && msg.contains("unexpected"), + "nested proxy unknown field should be rejected, got: {msg}" + ); + } + + #[test] + fn invalid_clipboard_rejected() { + // Strict enum: an out-of-range clipboard value is rejected at deserialize. + let json = r#"{"process": {"commandLine": "echo hi"}, "ui": {"clipboard": "bogus"}}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("unknown variant") && msg.contains("bogus"), + "invalid clipboard value should be rejected, got: {msg}" + ); + } + + #[test] + fn experimental_port_mapping_unknown_field_accepted() { + // The experimental surface is intentionally permissive (forward-compat): + // an unknown field on a nested experimental struct must be tolerated and + // the known fields preserved (positive proof of F2 / R2-5). + let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "wslc", "experimental": {"wslc": {"image": "python:3.12", "portMappings": [{"windowsPort": 8080, "containerPort": 80, "futureField": "ignored"}]}}}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true).unwrap(); + let wslc = req.experimental.wslc.expect("wslc config present"); + assert_eq!(wslc.port_mappings.len(), 1); + assert_eq!(wslc.port_mappings[0].windows_port, 8080); + assert_eq!(wslc.port_mappings[0].container_port, 80); + } + + #[test] + fn experimental_isolation_user_unknown_field_accepted() { + let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "isolation_session", "experimental": {"isolation_session": {"user": {"upn": "alice@contoso.com", "wamToken": "tok", "futureField": true}}}}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true).unwrap(); + let iso = req + .experimental + .isolation_session + .expect("iso config present"); + let user = iso.user.expect("user present"); + assert_eq!(user.upn, "alice@contoso.com"); + assert_eq!(user.wam_token, "tok"); + } + + #[test] + fn one_shot_rejects_phase_field() { + // A state-aware-shaped payload (carries `phase`) sent to a one-shot + // entry point must be rejected, not silently run as a one-shot. + let json = r#"{"process": {"commandLine": "echo hi"}, "phase": "provision"}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("phase") && msg.contains("state-aware"), + "one-shot path should reject 'phase', got: {msg}" + ); + } + + #[test] + fn one_shot_rejects_sandbox_id_field() { + let json = r#"{"process": {"commandLine": "echo hi"}, "sandboxId": "abc"}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("sandboxId") && msg.contains("state-aware"), + "one-shot path should reject 'sandboxId', got: {msg}" + ); + } + + #[test] + fn top_level_macos_sandbox_alias_maps_to_seatbelt() { + // The deprecated `macos_sandbox` section-key alias on the top-level + // `seatbelt` field is still accepted and maps to `req.seatbelt`. + let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "seatbelt", "macos_sandbox": {"guiAccess": true}}"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + + let req = load_request(&encoded, &mut logger, true).unwrap(); + let sb = req.seatbelt.expect("seatbelt config present via alias"); + assert!( + sb.gui_access, + "guiAccess should be carried through the alias" + ); + } + #[test] fn top_level_annotations_allowed() { // `$schema` and `_comment` are permitted but ignored. @@ -2911,6 +2689,114 @@ mod tests { ); } + #[test] + fn state_aware_rejects_one_shot_seatbelt_section() { + // A state-aware request carrying a one-shot-only `seatbelt` policy must + // be rejected, not silently discarded (the caller might believe the + // hardening is in effect). + let json = r#"{ + "phase": "provision", + "containment": "seatbelt", + "seatbelt": {"guiAccess": true} + }"#; + let err = match load_mxc(json) { + Err(ParseError::StateAware(e)) => e.to_string(), + other => panic!("expected StateAware rejection, got: {other:?}"), + }; + assert!( + err.contains("seatbelt") && err.contains("do not accept"), + "got: {err}" + ); + } + + #[test] + fn state_aware_rejects_one_shot_lifecycle_section() { + let json = r#"{ + "phase": "provision", + "containment": "isolation_session", + "lifecycle": {"destroyOnExit": false} + }"#; + let err = match load_mxc(json) { + Err(ParseError::StateAware(e)) => e.to_string(), + other => panic!("expected StateAware rejection, got: {other:?}"), + }; + assert!( + err.contains("lifecycle") && err.contains("do not accept"), + "got: {err}" + ); + } + + #[test] + fn state_aware_rejects_one_shot_processcontainer_section() { + let json = r#"{ + "phase": "provision", + "containment": "processcontainer", + "processContainer": {"leastPrivilege": true} + }"#; + let err = match load_mxc(json) { + Err(ParseError::StateAware(e)) => e.to_string(), + other => panic!("expected StateAware rejection, got: {other:?}"), + }; + assert!( + err.contains("processContainer") && err.contains("do not accept"), + "got: {err}" + ); + } + + #[test] + fn state_aware_rejects_one_shot_lxc_section() { + let json = r#"{ + "phase": "provision", + "containment": "lxc", + "lxc": {"distribution": "alpine"} + }"#; + let err = match load_mxc(json) { + Err(ParseError::StateAware(e)) => e.to_string(), + other => panic!("expected StateAware rejection, got: {other:?}"), + }; + assert!( + err.contains("lxc") && err.contains("do not accept"), + "got: {err}" + ); + } + + #[test] + fn state_aware_rejects_experimental_seatbelt() { + // `experimental.seatbelt` moved to the stable section; the state-aware + // path must reject it with the migration message, not silently discard + // it (R2-1 — the experimental-channel completion of F1). + let json = r#"{ + "phase": "provision", + "containment": "isolation_session", + "experimental": {"seatbelt": {"guiAccess": true}} + }"#; + let err = match load_mxc(json) { + Err(ParseError::StateAware(e)) => e.to_string(), + other => panic!("expected StateAware rejection, got: {other:?}"), + }; + assert!( + err.contains("has moved to the stable section"), + "got: {err}" + ); + } + + #[test] + fn state_aware_rejects_experimental_macos_sandbox_alias() { + let json = r#"{ + "phase": "provision", + "containment": "isolation_session", + "experimental": {"macos_sandbox": {"guiAccess": true}} + }"#; + let err = match load_mxc(json) { + Err(ParseError::StateAware(e)) => e.to_string(), + other => panic!("expected StateAware rejection, got: {other:?}"), + }; + assert!( + err.contains("has moved to the stable section"), + "got: {err}" + ); + } + #[test] fn state_aware_top_level_annotation_allowed() { let json = r#"{ @@ -3180,23 +3066,27 @@ mod tests { } #[test] - fn wslc_port_mapping_protocol_normalized_to_lowercase() { - // "TCP" is normalized to "tcp" and accepted. (UDP rejection is - // covered by wslc_port_mapping_udp_rejected_with_sdk_limitation_message.) + fn wslc_port_mapping_uppercase_protocol_rejected() { + // Strict enums are case-sensitive: "TCP" is not the lowercase wire + // value "tcp", so it is rejected at deserialize as an unknown variant. + // Only lowercase "tcp" is accepted (see wslc_port_mapping_basic_tcp_parsed). let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "wslc", "experimental": {"wslc": {"image": "python:3.12", "portMappings": [{"windowsPort": 8080, "containerPort": 80, "protocol": "TCP"}]}}}"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); - let req = load_request(&encoded, &mut logger, true).unwrap(); - let wslc = req.experimental.wslc.unwrap(); - assert_eq!(wslc.port_mappings[0].protocol, "tcp"); + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = format!("{}", err); + assert!( + msg.contains("unknown variant"), + "expected strict-enum rejection of uppercase protocol, got: {msg}" + ); } #[test] - fn wslc_port_mapping_udp_rejected_with_sdk_limitation_message() { - // The vendored WSLC SDK 2.8.1 runtime returns E_NOTIMPL for UDP. The - // parser must reject UDP up front with a clear message until a - // future SDK version implements it. + fn wslc_port_mapping_udp_rejected() { + // The wire model's TransportProtocol is tcp-only (the vendored WSLC SDK + // 2.8.1 runtime returns E_NOTIMPL for UDP), so "udp" is rejected at + // deserialize as an unknown enum variant. let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "wslc", "experimental": {"wslc": {"image": "python:3.12", "portMappings": [{"windowsPort": 5353, "containerPort": 53, "protocol": "udp"}]}}}"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); @@ -3204,7 +3094,7 @@ mod tests { let err = load_request(&encoded, &mut logger, true).unwrap_err(); let msg = format!("{}", err); assert!( - msg.contains("udp") && msg.contains("not supported"), + msg.contains("udp") && msg.contains("unknown variant"), "got: {msg}" ); } @@ -3267,6 +3157,8 @@ mod tests { #[test] fn wslc_port_mapping_unsupported_protocol_rejected() { + // An unknown protocol like "sctp" is rejected at deserialize: the + // tcp-only TransportProtocol enum has no matching variant. let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "wslc", "experimental": {"wslc": {"image": "python:3.12", "portMappings": [{"windowsPort": 8080, "containerPort": 80, "protocol": "sctp"}]}}}"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); @@ -3274,7 +3166,7 @@ mod tests { let err = load_request(&encoded, &mut logger, true).unwrap_err(); let msg = format!("{}", err); assert!( - msg.contains("sctp") && msg.contains("not supported"), + msg.contains("sctp") && msg.contains("unknown variant"), "got: {msg}" ); } @@ -3504,16 +3396,18 @@ mod tests { } #[test] - fn isolation_session_config_unknown_defaults_to_composable() { + fn isolation_session_config_unknown_is_rejected() { + // Strict enums: an unrecognized configurationId is rejected at + // deserialize time rather than silently defaulting to `composable`. let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "isolation_session", "experimental": {"isolation_session": {"configurationId": "xlarge"}}}"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); - let req = load_request(&encoded, &mut logger, true).unwrap(); - let cfg = req.experimental.isolation_session.unwrap(); - assert_eq!( - cfg.configuration_id, - crate::models::IsolationSessionConfigurationId::Composable + let err = load_request(&encoded, &mut logger, true).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("unknown variant") && msg.contains("xlarge"), + "expected an unknown-variant rejection for configurationId 'xlarge', got: {msg}" ); } diff --git a/src/core/wxc_common/src/lib.rs b/src/core/wxc_common/src/lib.rs index 9165e7441..3fefdfc18 100644 --- a/src/core/wxc_common/src/lib.rs +++ b/src/core/wxc_common/src/lib.rs @@ -22,10 +22,8 @@ pub mod state_aware_request; pub mod ui_policy; pub mod validator; -// Dedicated well-typed wire model + schema generation. Compiled under the -// `schema-gen` feature for now; it is intended to become the always-compiled -// parser deserialization target. -#[cfg(feature = "schema-gen")] +// Dedicated well-typed wire model. It is the parser's deserialization target; +// the JSON Schema is generated from it under the `schema-gen` feature. pub mod wire; // Thin Windows-only helpers that are not backend-specific. Backend diff --git a/src/core/wxc_common/src/state_aware_request.rs b/src/core/wxc_common/src/state_aware_request.rs index aa99d895b..a1c6f167a 100644 --- a/src/core/wxc_common/src/state_aware_request.rs +++ b/src/core/wxc_common/src/state_aware_request.rs @@ -43,22 +43,6 @@ impl Phase { Self::Deprovision => "deprovision", } } - - /// Parses the wire-format phase string. Unknown values produce - /// `MxcError::MalformedRequest`. - pub fn from_wire(s: &str) -> Result { - match s { - "provision" => Ok(Self::Provision), - "start" => Ok(Self::Start), - "exec" => Ok(Self::Exec), - "stop" => Ok(Self::Stop), - "deprovision" => Ok(Self::Deprovision), - other => Err(MxcError::malformed_request(format!( - "unknown phase {:?} (expected provision/start/exec/stop/deprovision)", - other - ))), - } - } } impl std::fmt::Display for Phase { @@ -162,26 +146,6 @@ mod tests { } } - #[test] - fn phase_from_wire_round_trip_for_all_variants() { - for p in [ - Phase::Provision, - Phase::Start, - Phase::Exec, - Phase::Stop, - Phase::Deprovision, - ] { - let parsed = Phase::from_wire(p.as_str()).unwrap(); - assert_eq!(parsed, p); - } - } - - #[test] - fn phase_from_wire_rejects_unknown() { - let err = Phase::from_wire("nope").unwrap_err(); - assert_eq!(err.code, MxcErrorCode::MalformedRequest); - } - #[test] fn deserialize_config_returns_none_when_no_experimental_block() { let p = parsed_with_experimental(None, Phase::Start); diff --git a/src/core/wxc_common/src/wire.rs b/src/core/wxc_common/src/wire.rs index 3e7dd1070..2e9351fd5 100644 --- a/src/core/wxc_common/src/wire.rs +++ b/src/core/wxc_common/src/wire.rs @@ -6,8 +6,7 @@ //! **generated** from these types via `mxc_schema_gen`; CI fails if the //! committed schema drifts (see `scripts/versioning/check-schema-codegen.js`). //! -//! Unlike the permissive `Raw*` deserialization structs in `config_parser`, -//! these types describe the config *contract* precisely: +//! These types describe the config *contract* precisely: //! //! * real `enum`s for closed value sets (`Containment`, `NetworkPolicy`, …) //! instead of `Option`, @@ -18,23 +17,24 @@ //! is intentionally left permissive (in-flux features), //! * `///` doc-comments that schemars turns into schema `description`s. //! -//! This module currently feeds schema generation only (compiled under the -//! `schema-gen` feature). It is intended to become the parser's actual -//! deserialization target, replacing the permissive `Raw*` structs and the -//! hand-written conversion in `config_parser`. +//! These types are the parser's actual deserialization target: `serde_json` +//! deserializes JSON into them, then `config_parser` maps them to the domain +//! `ExecutionRequest` / state-aware request. The `JsonSchema` derive is gated +//! behind the `schema-gen` feature so normal builds don't carry `schemars`; the +//! schema generator (`mxc_schema_gen`) enables it. //! //! Cross-field constraints (single-backend-section, phase-scoping) are NOT //! expressed in the generated schema; they are enforced by the parser, which is //! the trust boundary. The schema is an editor/CI convenience, never the gate. -use schemars::JsonSchema; use serde::{Deserialize, Serialize}; /// MXC container execution configuration. Defines the recommended config format -/// for both one-shot and state-aware sandbox lifecycle requests. The parser also -/// accepts legacy fields not listed here during the migration period. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[schemars(title = "MXC Configuration")] +/// for both one-shot and state-aware sandbox lifecycle requests. A few +/// deprecated field spellings not listed here are also accepted via serde aliases. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] +#[cfg_attr(feature = "schema-gen", schemars(title = "MXC Configuration"))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct MxcConfig { /// Optional JSON Schema reference for editor validation. Accepted but @@ -74,6 +74,7 @@ pub struct MxcConfig { /// ProcessContainer-specific settings (Windows). Used when containment is /// `processcontainer`. + #[serde(alias = "appContainer")] pub process_container: Option, /// LXC container settings (Linux). Used when containment is `lxc`. @@ -93,6 +94,7 @@ pub struct MxcConfig { /// macOS Seatbelt backend configuration. Used when containment is /// `seatbelt`. + #[serde(alias = "macos_sandbox")] pub seatbelt: Option, /// Experimental features. Only honored when `--experimental` is passed. @@ -100,7 +102,8 @@ pub struct MxcConfig { } /// State-aware lifecycle phase. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum Phase { Provision, @@ -111,13 +114,14 @@ pub enum Phase { } /// Containment backend (abstract intent or concrete backend). -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "snake_case")] pub enum Containment { /// OS-native process sandbox (resolved per host). Process, /// Windows AppContainer / BaseContainer. - #[serde(rename = "processcontainer")] + #[serde(rename = "processcontainer", alias = "appcontainer")] ProcessContainer, /// VM-class isolation (resolved per host). Vm, @@ -132,6 +136,7 @@ pub enum Containment { /// WSL container (experimental). Wslc, /// macOS Seatbelt. + #[serde(alias = "macos_sandbox")] Seatbelt, /// Windows IsolationSession (experimental). IsolationSession, @@ -140,7 +145,8 @@ pub enum Containment { } /// Process execution settings. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Process { /// Command line (or script) to execute. @@ -154,7 +160,8 @@ pub struct Process { } /// Container lifecycle settings. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Lifecycle { /// Destroy the container when the process exits (default true). @@ -164,7 +171,8 @@ pub struct Lifecycle { } /// ProcessContainer-specific settings. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct ProcessContainer { /// Enforce least-privilege mode. @@ -178,7 +186,8 @@ pub struct ProcessContainer { } /// BaseProcessContainer UI isolation settings. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct BaseProcessUi { /// UI isolation level. @@ -192,7 +201,8 @@ pub struct BaseProcessUi { } /// Desktop UI isolation level. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum UiIsolation { Desktop, @@ -202,7 +212,8 @@ pub enum UiIsolation { } /// LXC container settings. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Lxc { /// Distribution image (e.g. `alpine`). @@ -212,7 +223,8 @@ pub struct Lxc { } /// Filesystem access policy. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Filesystem { /// Paths the process can read and write. @@ -224,7 +236,8 @@ pub struct Filesystem { } /// AppContainer DACL-mutation fallback policy. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Fallback { /// Allow the runner to mutate DACLs as a fallback. @@ -232,7 +245,8 @@ pub struct Fallback { } /// Network access policy. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Network { /// Default outbound policy when no host rule matches. @@ -250,7 +264,8 @@ pub struct Network { } /// Default network policy. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum NetworkPolicy { Allow, @@ -258,7 +273,8 @@ pub enum NetworkPolicy { } /// Network enforcement mechanism. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum NetworkEnforcement { /// Per-process capability-based filtering. @@ -270,11 +286,12 @@ pub enum NetworkEnforcement { } /// Proxy configuration. Exactly one variant applies. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Proxy { /// External localhost proxy port. - #[schemars(range(min = 1, max = 65535))] + #[cfg_attr(feature = "schema-gen", schemars(range(min = 1, max = 65535)))] pub localhost: Option, /// Have wxc launch its own built-in test proxy. pub builtin_test_server: Option, @@ -283,7 +300,8 @@ pub struct Proxy { } /// Cross-platform UI isolation policy. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Ui { /// Disable all UI access (default true). @@ -295,7 +313,8 @@ pub struct Ui { } /// Clipboard access level. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum ClipboardPolicy { None, @@ -305,7 +324,8 @@ pub enum ClipboardPolicy { } /// macOS Seatbelt backend configuration. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Seatbelt { /// Replace the generated profile entirely (advanced/testing escape hatch). @@ -323,7 +343,8 @@ pub struct Seatbelt { } /// Seatbelt inner-process launch method. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum LaunchMethod { /// sandbox_init() + exec (default). Works for third-party GUI apps. @@ -338,7 +359,8 @@ pub enum LaunchMethod { /// backends are in flux, so the schema documents the known shapes for editor /// help without rejecting in-progress fields. The strict, closed contract is /// the stable (top-level) surface. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] pub struct Experimental { /// Placeholder feature for testing experimental infrastructure. pub test: Option, @@ -349,11 +371,13 @@ pub struct Experimental { /// IsolationSession backend config (Windows). pub isolation_session: Option, /// Seatbelt backend config (pre-promotion alias). + #[serde(alias = "macos_sandbox")] pub seatbelt: Option, } /// Placeholder experimental feature. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] pub struct TestFeature { /// Message to log when the feature is applied. @@ -361,7 +385,8 @@ pub struct TestFeature { } /// Windows Sandbox backend config. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] pub struct WindowsSandbox { /// Idle timeout before teardown (ms). @@ -373,7 +398,8 @@ pub struct WindowsSandbox { } /// WSL container backend config. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] pub struct Wslc { /// OS inside the WSL container. @@ -396,15 +422,17 @@ pub struct Wslc { pub port_mappings: Option>, } -/// A single host → container port forward. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "camelCase", deny_unknown_fields)] +/// A single host → container port forward. Reachable only under the permissive +/// `experimental` surface, so unknown fields are tolerated (forward-compat). +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] pub struct PortMapping { /// Host (Windows) port. - #[schemars(range(min = 1, max = 65535))] + #[cfg_attr(feature = "schema-gen", schemars(range(min = 1, max = 65535)))] pub windows_port: u16, /// Container port. - #[schemars(range(min = 1, max = 65535))] + #[cfg_attr(feature = "schema-gen", schemars(range(min = 1, max = 65535)))] pub container_port: u16, /// Transport protocol for the mapping. Only `tcp` is currently supported. pub protocol: Option, @@ -412,7 +440,8 @@ pub struct PortMapping { /// Port-forward transport protocol. Only `tcp` is currently supported by the /// vendored WSLC SDK runtime; `udp` is rejected at parse time. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum TransportProtocol { Tcp, @@ -421,7 +450,8 @@ pub enum TransportProtocol { /// IsolationSession backend config. Carries both the one-shot fields /// (`configurationId`, `user`) and the per-phase state-aware nesting /// (`provision` / `start` / `stop` / `deprovision`). -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] pub struct IsolationSession { /// Sizing profile (one-shot). @@ -439,7 +469,8 @@ pub struct IsolationSession { } /// Per-phase IsolationSession configuration (state-aware lifecycle). -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] pub struct IsolationSessionPhase { /// Sizing profile for this phase. @@ -449,7 +480,8 @@ pub struct IsolationSessionPhase { } /// IsolationSession sizing profile. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] #[serde(rename_all = "lowercase")] pub enum IsolationConfigurationId { Small, @@ -458,9 +490,11 @@ pub enum IsolationConfigurationId { Composable, } -/// Entra cloud-agent user bundle. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "camelCase", deny_unknown_fields)] +/// Entra cloud-agent user bundle. Reachable only under the permissive +/// `experimental` surface, so unknown fields are tolerated (forward-compat). +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] pub struct IsolationUser { /// User principal name. pub upn: String, @@ -470,6 +504,7 @@ pub struct IsolationUser { /// Canonical `$id` for the generated dev schema. Bump alongside the dev schema /// version/filename (see `schemas/schema-version.json`). +#[cfg(feature = "schema-gen")] const SCHEMA_ID: &str = "https://github.com/microsoft/mxc/schemas/dev/mxc-config.schema.0.8.0-dev.json"; @@ -482,6 +517,7 @@ const SCHEMA_ID: &str = /// (`$schema`, `$id`, `title`, `description`) first for readability. `title` and /// `description` come from the `MxcConfig` schemars attribute / doc comment /// respectively. +#[cfg(feature = "schema-gen")] pub fn generate_config_schema_json() -> String { let schema = schemars::schema_for!(MxcConfig); let mut value = serde_json::to_value(&schema).expect("schema serialises to JSON value"); @@ -507,6 +543,7 @@ pub fn generate_config_schema_json() -> String { /// serializer (so nested objects stay alphabetical, byte-for-byte as before) and /// re-indented one level. Any key not in `ORDER` keeps its alphabetical position /// after the listed ones. +#[cfg(feature = "schema-gen")] fn render_root_ordered(map: &serde_json::Map) -> String { // Only the metadata keys are floated to the front; every other key keeps its // natural (alphabetical) position, so the rest of the file is unchanged. @@ -548,6 +585,7 @@ fn render_root_ordered(map: &serde_json::Map) -> Stri /// constructs: unsigned types (`uint*`) gain `minimum: 0` and drop `format`; /// signed types (`int*`) just drop `format`. Standard string formats /// (`date-time`, `uri`, …) are left untouched. +#[cfg(feature = "schema-gen")] fn normalize_integer_formats(value: &mut serde_json::Value) { use serde_json::Value; match value { From 428deaa64572881e2be5cf64be9ff9db3edb7dad Mon Sep 17 00:00:00 2001 From: Gudge Date: Thu, 25 Jun 2026 09:23:45 -0700 Subject: [PATCH 2/3] Phase 2B: address review feedback (From impls, schema-gen submodule, doc cleanup) This change applies code-review feedback on the wire-model parser without altering behavior: it makes the trivial wire->domain enum/struct conversions idiomatic, groups the schema-generation code behind one feature gate, and removes stale references to the deleted Raw* structs. Details: - Replace the free `map_wire_*` enum/struct converters with `From` impls beside the domain types: `From`, `From`, `From` in models.rs and `From` in state_aware_request.rs; call sites now use `.into()`. `map_wire_containment` stays a function (it resolves `Option<&_>` per target_os, not a pure conversion). - Group the schema-gen functions (`generate_config_schema_json` and its helpers) into a single `#[cfg(feature = "schema-gen")] mod schema_gen`, re-exporting the one public entry point, instead of repeating the attribute per item. The wire structs stay un-gated (they are the parser's deserialization target); only the schemars-dependent generation code is gated. - Rename the discriminator `value` to `parsed_json` in the parse entry point. - Drop the now-redundant SDK comments explaining omitted wire fields, and remove stale `Raw*` references from docs (authoring-a-new-feature, bubblewrap plan, state-aware API, copilot-instructions). Tests: - cargo build -p wxc_common (default + schema-gen); cargo clippy --workspace --all-targets -- -D warnings; cargo fmt --all -- --check. - cargo test --workspace; schema codegen byte-identical; corpus 169/169. - cd sdk && npm test -> 183 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8 --- .github/copilot-instructions.md | 2 +- docs/authoring-a-new-feature.md | 6 +- docs/bwrap-support/bubblewrap-backend-plan.md | 6 - .../mxc-state-aware-sandbox-api.md | 6 +- sdk/src/sandbox.ts | 6 - src/core/wxc_common/src/config_parser.rs | 72 ++----- src/core/wxc_common/src/models.rs | 28 +++ .../wxc_common/src/state_aware_request.rs | 12 ++ src/core/wxc_common/src/wire.rs | 203 +++++++++--------- 9 files changed, 168 insertions(+), 173 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e75c52deb..9624b8926 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -163,7 +163,7 @@ State-aware lifecycle: New features go under the `experimental` JSON section and are only active when `--experimental` is passed. See `docs/authoring-a-new-feature.md` for the full checklist. The pattern: 1. Add the field to the Rust wire model (`src/core/wxc_common/src/wire.rs`) under the `Experimental` section, then regenerate the dev schema (`cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- schemas/dev/mxc-config.schema..json`) — do not hand-edit the generated schema -2. Add the matching field to the wire model's `Experimental` struct (`src/core/wxc_common/src/wire.rs`) and the domain `ExperimentalConfig` in `models.rs`, then map wire→domain in `config_parser.rs` +2. Add the matching field to the wire model's `Experimental` struct (`src/core/wxc_common/src/wire.rs`) and the domain `ExperimentalConfig` in `models.rs`, then map wire→domain in `config_parser.rs` (use `From` impls beside the domain type for trivial enum/struct conversions) 3. Guard execution behind `if request.experimental_enabled` in the runner 4. Never modify files in `schemas/stable/` — those are immutable release artifacts diff --git a/docs/authoring-a-new-feature.md b/docs/authoring-a-new-feature.md index ca31d35af..04017750b 100644 --- a/docs/authoring-a-new-feature.md +++ b/docs/authoring-a-new-feature.md @@ -186,9 +186,9 @@ pub struct ExperimentalConfig { ## Step 3: Map the wire field to the domain model The parser deserializes JSON directly into the wire model (`wire::MxcConfig`), -so there is no separate `Raw*` struct to add — your `wire::GpuIsolation` from -Step 1 is the parse target. In `config_parser.rs`, map it to the domain struct -inside `convert_wire_config` where the `experimental` block is converted: +so your `wire::GpuIsolation` from Step 1 is the parse target. In +`config_parser.rs`, map it to the domain struct inside `convert_wire_config` +where the `experimental` block is converted: ```rust let experimental = if let Some(raw_exp) = cfg.experimental { diff --git a/docs/bwrap-support/bubblewrap-backend-plan.md b/docs/bwrap-support/bubblewrap-backend-plan.md index f17b06aed..02c0b8630 100644 --- a/docs/bwrap-support/bubblewrap-backend-plan.md +++ b/docs/bwrap-support/bubblewrap-backend-plan.md @@ -75,12 +75,6 @@ A backend-specific config can be added later under `ExperimentalConfig` if neede ### 3. Config Parser Changes -> **Note (superseded):** This plan predates the wire-model parser rewire. The -> parser no longer uses `Raw*` structs or `convert_raw_config_inner`; it -> deserializes into `wire::MxcConfig` (`src/core/wxc_common/src/wire.rs`) and maps -> to the domain model in `convert_wire_config`. Translate the steps below to the -> wire model — see [authoring-a-new-feature.md](../authoring-a-new-feature.md). - **File:** `src/core/wxc_common/src/wire.rs` and `config_parser.rs` - Add a `Bubblewrap` variant to the wire `Containment` enum (or rely on the diff --git a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md index 4445fba3a..438a9aac1 100644 --- a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md +++ b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md @@ -985,9 +985,9 @@ implements one trait, the other, or both, depending on its declared participatio MXC's parser at `src/core/wxc_common/src/config_parser.rs` deserializes the wire-format JSON directly into the typed wire model in `src/core/wxc_common/src/wire.rs` (`wire::MxcConfig`), then maps it into the -typed domain models via `convert_*` helpers (`convert_wire_config` → -`ExecutionRequest`) before dispatch. The state-aware path reuses this same wire -model. +typed domain models (`convert_wire_config` → `ExecutionRequest`, with `From` +impls beside the domain types for the trivial enum/struct conversions) before +dispatch. The state-aware path reuses this same wire model. ```rust // In config_parser.rs — discrimination is by presence of the `phase` key in diff --git a/sdk/src/sandbox.ts b/sdk/src/sandbox.ts index 6c730deb2..f7d397637 100644 --- a/sdk/src/sandbox.ts +++ b/sdk/src/sandbox.ts @@ -106,9 +106,6 @@ function buildBubblewrapConfig( function buildLinuxProcessConfig( config: ContainerConfig, ): ContainerConfig { - // The container id is carried at top-level `containerId` and the teardown - // flag at `lifecycle.destroyOnExit`; the wire `lxc` object has only - // `distribution` / `release`, so do not duplicate those here. config.lxc = { distribution: 'alpine', release: '3.23', @@ -149,9 +146,6 @@ function buildProcessBaseContainerConfig( capabilities.push("privateNetworkClientServer"); } - // The container id is carried only at the top level (`containerId`, set by - // createConfigFromPolicy); the wire `processContainer` object has no `name` - // field, so emitting it here would be rejected by the parser. config.processContainer = { leastPrivilege: false, capabilities, diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 51348d651..dada24a56 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -9,9 +9,9 @@ use crate::error::WxcError; use crate::logger::Logger; use crate::models::{ ClipboardPolicy, ContainerPolicy, ContainmentBackend, ExecutionRequest, ExperimentalConfig, - IsolationSessionConfig, IsolationSessionConfigurationId, IsolationSessionUser, LifecycleConfig, - LxcConfig, NetworkEnforcementMode, NetworkPolicy, PortMapping, ProxyAddress, ProxyConfig, - SeatbeltConfig, TestFeatureConfig, UiPolicy, WindowsSandboxConfig, WslcConfig, + IsolationSessionConfig, LifecycleConfig, LxcConfig, NetworkEnforcementMode, NetworkPolicy, + PortMapping, ProxyAddress, ProxyConfig, SeatbeltConfig, TestFeatureConfig, UiPolicy, + WindowsSandboxConfig, WslcConfig, }; use crate::mxc_error::MxcError; use crate::state_aware_request::{MxcRequest, ParsedStateAwareRequest, Phase}; @@ -138,24 +138,24 @@ pub fn load_mxc_request_with_options( let json_str = decode_request_input(input, logger, opts.is_base64).map_err(ParseError::Decode)?; - // Parse once into a generic value so we can (a) discriminate one-shot vs - // state-aware by presence of the `phase` key and (b) capture the raw + // Parse once into a generic JSON value so we can (a) discriminate one-shot + // vs state-aware by presence of the `phase` key and (b) capture the raw // `experimental` block for the state-aware path, where it is typed // per-backend at dispatch time rather than at parse time. - let value: serde_json::Value = serde_json::from_str(&json_str).map_err(|e| { + let parsed_json: serde_json::Value = serde_json::from_str(&json_str).map_err(|e| { logger.log_line("Error parsing JSON"); ParseError::Decode(WxcError::ConfigParse(format!("JSON parse error: {}", e))) })?; - if value.get("phase").is_some() { - convert_wire_state_aware(value, logger, opts.allow_missing_command) + if parsed_json.get("phase").is_some() { + convert_wire_state_aware(parsed_json, logger, opts.allow_missing_command) .map(MxcRequest::StateAware) .map_err(|e| ParseError::StateAware(MxcError::malformed_request(e.to_string()))) } else { - // Re-deserialize from the source text (not the parsed `Value`) so - // serde's line/column context is preserved in error messages on this - // trust boundary; `from_value` discards it, turning a typo or - // out-of-range field into an unlocalised "expected u16"-style dump. + // Re-deserialize from the source text (not the already-parsed + // `parsed_json`) so serde's line/column context is preserved in error + // messages on this trust boundary; `from_value` discards it, turning a + // typo or out-of-range field into an unlocalised "expected u16"-style dump. let cfg: wire::MxcConfig = serde_json::from_str(&json_str).map_err(|e| { logger.log_line("Error parsing JSON"); ParseError::OneShot(WxcError::ConfigParse(format!("JSON parse error: {}", e))) @@ -500,23 +500,13 @@ fn make_seatbelt_config(sb: wire::Seatbelt) -> SeatbeltConfig { SeatbeltConfig { profile_override, gui_access: gui_access.unwrap_or(false), - launch_method: launch_method - .map(map_wire_launch_method) - .unwrap_or_default(), + launch_method: launch_method.map(Into::into).unwrap_or_default(), nested_pty: nested_pty.unwrap_or(true), keychain_access: keychain_access.unwrap_or(false), extra_mach_lookups: extra_mach_lookups.unwrap_or_default(), } } -/// Map the wire Seatbelt launch method to the domain enum. -fn map_wire_launch_method(m: wire::LaunchMethod) -> crate::models::LaunchMethod { - match m { - wire::LaunchMethod::Exec => crate::models::LaunchMethod::Exec, - wire::LaunchMethod::Open => crate::models::LaunchMethod::Open, - } -} - /// Lowercase wire-format string for a UI isolation level (matches the schema /// enum values consumed downstream). fn wire_ui_isolation_str(iso: &wire::UiIsolation) -> &'static str { @@ -528,36 +518,6 @@ fn wire_ui_isolation_str(iso: &wire::UiIsolation) -> &'static str { } } -/// Map the wire lifecycle phase to the domain `Phase`. -fn map_wire_phase(p: wire::Phase) -> Phase { - match p { - wire::Phase::Provision => Phase::Provision, - wire::Phase::Start => Phase::Start, - wire::Phase::Exec => Phase::Exec, - wire::Phase::Stop => Phase::Stop, - wire::Phase::Deprovision => Phase::Deprovision, - } -} - -/// Map the wire IsolationSession sizing profile to the domain enum. -fn map_wire_isolation_configuration_id( - id: wire::IsolationConfigurationId, -) -> IsolationSessionConfigurationId { - match id { - wire::IsolationConfigurationId::Small => IsolationSessionConfigurationId::Small, - wire::IsolationConfigurationId::Medium => IsolationSessionConfigurationId::Medium, - wire::IsolationConfigurationId::Large => IsolationSessionConfigurationId::Large, - wire::IsolationConfigurationId::Composable => IsolationSessionConfigurationId::Composable, - } -} - -/// Map the wire Entra user bundle to the domain struct. -fn map_wire_isolation_user(u: wire::IsolationUser) -> IsolationSessionUser { - // Destructure (no `..`) so a new wire field fails to compile until mapped. - let wire::IsolationUser { upn, wam_token } = u; - IsolationSessionUser { upn, wam_token } -} - /// Resolve the `containment` wire enum to a concrete domain backend. /// /// `None` (omitted) and the abstract intent `Process` resolve to the OS-native @@ -977,9 +937,9 @@ fn convert_wire_config( let isolation_session = raw_exp.isolation_session.map(|as_cfg| { let mut config = IsolationSessionConfig::default(); if let Some(id) = as_cfg.configuration_id { - config.configuration_id = map_wire_isolation_configuration_id(id); + config.configuration_id = id.into(); } - config.user = as_cfg.user.map(map_wire_isolation_user); + config.user = as_cfg.user.map(Into::into); config }); if raw_exp.seatbelt.is_some() { @@ -1056,7 +1016,7 @@ fn convert_wire_state_aware( // `phase` is the state-aware discriminator and is constrained by the wire // enum; absence here would be a logic error in the caller's discrimination. let phase = match cfg.phase.take() { - Some(p) => map_wire_phase(p), + Some(p) => p.into(), None => { return Err(WxcError::ConfigParse( "Missing required field: phase".to_string(), diff --git a/src/core/wxc_common/src/models.rs b/src/core/wxc_common/src/models.rs index f1ff02280..279f70050 100644 --- a/src/core/wxc_common/src/models.rs +++ b/src/core/wxc_common/src/models.rs @@ -173,6 +173,15 @@ pub enum LaunchMethod { Open, } +impl From for LaunchMethod { + fn from(m: crate::wire::LaunchMethod) -> Self { + match m { + crate::wire::LaunchMethod::Exec => Self::Exec, + crate::wire::LaunchMethod::Open => Self::Open, + } + } +} + /// Configuration specific to the Windows Sandbox backend. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default)] @@ -212,6 +221,17 @@ pub enum IsolationSessionConfigurationId { Composable, } +impl From for IsolationSessionConfigurationId { + fn from(id: crate::wire::IsolationConfigurationId) -> Self { + match id { + crate::wire::IsolationConfigurationId::Small => Self::Small, + crate::wire::IsolationConfigurationId::Medium => Self::Medium, + crate::wire::IsolationConfigurationId::Large => Self::Large, + crate::wire::IsolationConfigurationId::Composable => Self::Composable, + } + } +} + /// Configuration specific to the Isolation Session backend. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(default)] @@ -243,6 +263,14 @@ impl std::fmt::Debug for IsolationSessionUser { } } +impl From for IsolationSessionUser { + fn from(u: crate::wire::IsolationUser) -> Self { + // Destructure (no `..`) so a new wire field fails to compile until mapped. + let crate::wire::IsolationUser { upn, wam_token } = u; + Self { upn, wam_token } + } +} + /// State-aware provision-phase config for the Isolation Session backend. /// Nested under `experimental.isolation_session.provision`. Carries Entra /// credentials when the caller wants a cloud-agent sandbox; absent for diff --git a/src/core/wxc_common/src/state_aware_request.rs b/src/core/wxc_common/src/state_aware_request.rs index a1c6f167a..3932d48e4 100644 --- a/src/core/wxc_common/src/state_aware_request.rs +++ b/src/core/wxc_common/src/state_aware_request.rs @@ -51,6 +51,18 @@ impl std::fmt::Display for Phase { } } +impl From for Phase { + fn from(p: crate::wire::Phase) -> Self { + match p { + crate::wire::Phase::Provision => Self::Provision, + crate::wire::Phase::Start => Self::Start, + crate::wire::Phase::Exec => Self::Exec, + crate::wire::Phase::Stop => Self::Stop, + crate::wire::Phase::Deprovision => Self::Deprovision, + } + } +} + /// Parsed state-aware request. Pairs the inner `ExecutionRequest` with the /// state-aware-only wire fields the dispatcher consumes. #[derive(Debug, Clone)] diff --git a/src/core/wxc_common/src/wire.rs b/src/core/wxc_common/src/wire.rs index 2e9351fd5..f0934c1b4 100644 --- a/src/core/wxc_common/src/wire.rs +++ b/src/core/wxc_common/src/wire.rs @@ -502,114 +502,121 @@ pub struct IsolationUser { pub wam_token: String, } -/// Canonical `$id` for the generated dev schema. Bump alongside the dev schema -/// version/filename (see `schemas/schema-version.json`). +/// JSON Schema generation from the wire model, gated behind `schema-gen` so +/// production builds don't carry `schemars`. The single public entry point is +/// re-exported below as `generate_config_schema_json`. #[cfg(feature = "schema-gen")] -const SCHEMA_ID: &str = - "https://github.com/microsoft/mxc/schemas/dev/mxc-config.schema.0.8.0-dev.json"; - -/// Generate the JSON Schema for the MXC config from the dedicated `MxcConfig` -/// model. The schema is post-processed to (a) inject the canonical `$id`, -/// (b) replace schemars' Rust-specific integer `format` strings (`uint32`, -/// `int64`, …) — which JSON Schema draft-07 does not define — with standard -/// constraints (`minimum: 0` for unsigned), so the committed artifact validates -/// cleanly under standard tooling, and (c) emit the root metadata keys -/// (`$schema`, `$id`, `title`, `description`) first for readability. `title` and -/// `description` come from the `MxcConfig` schemars attribute / doc comment -/// respectively. -#[cfg(feature = "schema-gen")] -pub fn generate_config_schema_json() -> String { - let schema = schemars::schema_for!(MxcConfig); - let mut value = serde_json::to_value(&schema).expect("schema serialises to JSON value"); - normalize_integer_formats(&mut value); - if let serde_json::Value::Object(map) = &mut value { - map.insert( - "$id".to_string(), - serde_json::Value::String(SCHEMA_ID.to_string()), - ); - return render_root_ordered(map); +mod schema_gen { + use super::MxcConfig; + + /// Canonical `$id` for the generated dev schema. Bump alongside the dev schema + /// version/filename (see `schemas/schema-version.json`). + const SCHEMA_ID: &str = + "https://github.com/microsoft/mxc/schemas/dev/mxc-config.schema.0.8.0-dev.json"; + + /// Generate the JSON Schema for the MXC config from the dedicated `MxcConfig` + /// model. The schema is post-processed to (a) inject the canonical `$id`, + /// (b) replace schemars' Rust-specific integer `format` strings (`uint32`, + /// `int64`, …) — which JSON Schema draft-07 does not define — with standard + /// constraints (`minimum: 0` for unsigned), so the committed artifact validates + /// cleanly under standard tooling, and (c) emit the root metadata keys + /// (`$schema`, `$id`, `title`, `description`) first for readability. `title` and + /// `description` come from the `MxcConfig` schemars attribute / doc comment + /// respectively. + pub fn generate_config_schema_json() -> String { + let schema = schemars::schema_for!(MxcConfig); + let mut value = serde_json::to_value(&schema).expect("schema serialises to JSON value"); + normalize_integer_formats(&mut value); + if let serde_json::Value::Object(map) = &mut value { + map.insert( + "$id".to_string(), + serde_json::Value::String(SCHEMA_ID.to_string()), + ); + return render_root_ordered(map); + } + serde_json::to_string_pretty(&value).expect("schema serialises to JSON") } - serde_json::to_string_pretty(&value).expect("schema serialises to JSON") -} -/// Render the root object as pretty JSON with a fixed key order — the schema -/// metadata (`$schema`, `$id`, `title`, `description`) first, then the -/// structural keys — without disturbing nested key order. -/// -/// `serde_json`'s default `Map` is a `BTreeMap`, so it emits every object's keys -/// alphabetically and gives no control over root order. Rather than switch the -/// whole crate to `preserve_order` (which would reorder every nested object too), -/// only the root is rendered here: each value is pretty-printed with the standard -/// serializer (so nested objects stay alphabetical, byte-for-byte as before) and -/// re-indented one level. Any key not in `ORDER` keeps its alphabetical position -/// after the listed ones. -#[cfg(feature = "schema-gen")] -fn render_root_ordered(map: &serde_json::Map) -> String { - // Only the metadata keys are floated to the front; every other key keeps its - // natural (alphabetical) position, so the rest of the file is unchanged. - const ORDER: &[&str] = &["$schema", "$id", "title", "description"]; - let rank = |key: &str| ORDER.iter().position(|k| *k == key).unwrap_or(ORDER.len()); - - // `map` is a BTreeMap, so `keys()` is already alphabetical; a stable sort by - // rank floats the listed keys to the front and leaves the rest alphabetical. - let mut keys: Vec<&String> = map.keys().collect(); - keys.sort_by_key(|k| rank(k)); - - let mut out = String::from("{\n"); - for (i, key) in keys.iter().enumerate() { - let value_pretty = - serde_json::to_string_pretty(&map[*key]).expect("schema value serialises to JSON"); - // The value sits one level deep: keep its first line in place after the - // key, and indent every following line by two spaces. - let mut lines = value_pretty.lines(); - let mut indented = lines.next().unwrap_or("").to_string(); - for line in lines { - indented.push_str("\n "); - indented.push_str(line); - } - let key_json = serde_json::to_string(key).expect("object key serialises to JSON"); - out.push_str(" "); - out.push_str(&key_json); - out.push_str(": "); - out.push_str(&indented); - if i + 1 < keys.len() { - out.push(','); + /// Render the root object as pretty JSON with a fixed key order — the schema + /// metadata (`$schema`, `$id`, `title`, `description`) first, then the + /// structural keys — without disturbing nested key order. + /// + /// `serde_json`'s default `Map` is a `BTreeMap`, so it emits every object's keys + /// alphabetically and gives no control over root order. Rather than switch the + /// whole crate to `preserve_order` (which would reorder every nested object too), + /// only the root is rendered here: each value is pretty-printed with the standard + /// serializer (so nested objects stay alphabetical, byte-for-byte as before) and + /// re-indented one level. Any key not in `ORDER` keeps its alphabetical position + /// after the listed ones. + fn render_root_ordered(map: &serde_json::Map) -> String { + // Only the metadata keys are floated to the front; every other key keeps its + // natural (alphabetical) position, so the rest of the file is unchanged. + const ORDER: &[&str] = &["$schema", "$id", "title", "description"]; + let rank = |key: &str| ORDER.iter().position(|k| *k == key).unwrap_or(ORDER.len()); + + // `map` is a BTreeMap, so `keys()` is already alphabetical; a stable sort by + // rank floats the listed keys to the front and leaves the rest alphabetical. + let mut keys: Vec<&String> = map.keys().collect(); + keys.sort_by_key(|k| rank(k)); + + let mut out = String::from("{\n"); + for (i, key) in keys.iter().enumerate() { + let value_pretty = + serde_json::to_string_pretty(&map[*key]).expect("schema value serialises to JSON"); + // The value sits one level deep: keep its first line in place after the + // key, and indent every following line by two spaces. + let mut lines = value_pretty.lines(); + let mut indented = lines.next().unwrap_or("").to_string(); + for line in lines { + indented.push_str("\n "); + indented.push_str(line); + } + let key_json = serde_json::to_string(key).expect("object key serialises to JSON"); + out.push_str(" "); + out.push_str(&key_json); + out.push_str(": "); + out.push_str(&indented); + if i + 1 < keys.len() { + out.push(','); + } + out.push('\n'); } - out.push('\n'); + out.push('}'); + out } - out.push('}'); - out -} -/// Recursively rewrite non-standard schemars integer `format`s into draft-07 -/// constructs: unsigned types (`uint*`) gain `minimum: 0` and drop `format`; -/// signed types (`int*`) just drop `format`. Standard string formats -/// (`date-time`, `uri`, …) are left untouched. -#[cfg(feature = "schema-gen")] -fn normalize_integer_formats(value: &mut serde_json::Value) { - use serde_json::Value; - match value { - Value::Object(map) => { - if let Some(Value::String(fmt)) = map.get("format") { - let fmt = fmt.clone(); - let is_unsigned = fmt.starts_with("uint"); - let is_signed = fmt.starts_with("int"); - if is_unsigned || is_signed { - map.remove("format"); - if is_unsigned { - map.entry("minimum").or_insert(Value::Number(0.into())); + /// Recursively rewrite non-standard schemars integer `format`s into draft-07 + /// constructs: unsigned types (`uint*`) gain `minimum: 0` and drop `format`; + /// signed types (`int*`) just drop `format`. Standard string formats + /// (`date-time`, `uri`, …) are left untouched. + fn normalize_integer_formats(value: &mut serde_json::Value) { + use serde_json::Value; + match value { + Value::Object(map) => { + if let Some(Value::String(fmt)) = map.get("format") { + let fmt = fmt.clone(); + let is_unsigned = fmt.starts_with("uint"); + let is_signed = fmt.starts_with("int"); + if is_unsigned || is_signed { + map.remove("format"); + if is_unsigned { + map.entry("minimum").or_insert(Value::Number(0.into())); + } } } + for v in map.values_mut() { + normalize_integer_formats(v); + } } - for v in map.values_mut() { - normalize_integer_formats(v); - } - } - Value::Array(items) => { - for v in items { - normalize_integer_formats(v); + Value::Array(items) => { + for v in items { + normalize_integer_formats(v); + } } + _ => {} } - _ => {} } } + +#[cfg(feature = "schema-gen")] +pub use schema_gen::generate_config_schema_json; From 606d59f2aad2f9e05f1cc6c2f9b4b12025074310 Mon Sep 17 00:00:00 2001 From: Gudge Date: Thu, 25 Jun 2026 09:43:40 -0700 Subject: [PATCH 3/3] Phase 2B: document the wire-model vs runtime-model split This change adds a "Wire Model vs Runtime Model" section to docs/versioning.md explaining why the parser keeps two Rust representations (the JSON-mirroring wire model and the validated runtime model) with a mapping at the parse boundary, rather than a single shared type. Details: - Describe the wire model (schema/SDK codegen source, faithful JSON shape) and the runtime/domain model (validated, invariant-rich, what backends consume), and the parser as the single validate/normalize boundary between them. - Lay out the pros (one boundary, parse-don't-validate, pure schema source, decoupled evolution) and cons (boilerplate, internal drift, indirection), and note the costs are addressable without merging the layers. Tests: - Documentation-only change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8 --- docs/versioning.md | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/docs/versioning.md b/docs/versioning.md index 19ed2abc2..c82c0d023 100644 --- a/docs/versioning.md +++ b/docs/versioning.md @@ -251,6 +251,77 @@ MXC calls OS: CreateProcessInSandbox(flatbuffer) Process runs in sandbox ``` +## Wire Model vs Runtime Model + +MXC deliberately keeps **two** Rust representations of a config with a mapping at +the parse boundary, rather than one shared type: + +- **Wire model** (`wxc_common::wire::MxcConfig`) — a faithful 1:1 mirror of the + config JSON: every field `Option`, `camelCase`, `experimental` carried as a raw + `serde_json::Value`, no invariants enforced. It is the parser's deserialization + target **and** the single source of truth the JSON schema (via schemars) and the + SDK TypeScript types are generated from. +- **Runtime / domain model** (`models::ExecutionRequest` and friends) — the + validated, defaults-applied, invariant-rich model the backends consume: + abstract containment resolved to a concrete backend, `process.commandLine` + reshaped to `script_code`, enums resolved to domain enums, required fields no + longer `Option`. + +The parser (`config_parser`) is the one validate/normalize boundary between them; +trivial enum/struct conversions are `From` impls beside the domain type, and the +larger reshaping lives in `convert_wire_config`. + +### Why two layers (pros) + +- **One validate/normalize boundary.** Defaults, invariant enforcement, + abstract→concrete backend resolution, and field reshaping all happen in exactly + one place; backends receive a type whose invariants already hold. +- **Parse, don't validate.** The domain type makes illegal states + unrepresentable (required fields non-`Option`, enums resolved, containment + always concrete), so a backend never re-checks "is this set / known?". +- **The wire model stays a pure schema/DTO source.** Being exactly the JSON shape + is what makes schemars-from-types and SDK TS codegen clean — and it is what the + per-field stability attributes (stable/experimental/deprecated, for the + stable-vs-dev schema views and the promotion guard) hang on. A merged type would + entangle schema-generation concerns with runtime fields. +- **Decoupled evolution.** The wire format can change (rename, alias, restructure + `experimental`) without touching backend code, and vice-versa; the blast radius + of either is bounded by the parser. +- **Backends don't couple to JSON quirks** — camelCase renames, deprecated-spelling + serde aliases, the raw-`Value` experimental block, `$schema`/`_comment` + passthrough — none leak into runner code. + +### Costs (cons) + +- **Boilerplate.** Two definitions plus a mapping for each object; adding a field + touches the wire struct, the domain struct, and the parser (`From` impls only + soften the trivial cases). +- **Internal drift risk.** The two Rust types can fall out of sync. This is + mitigated by destructuring wire structs without `..` in conversions (a new wire + field then fails to compile until mapped), but that is a convention, not a + guarantee everywhere. +- **Indirection.** Tracing one field means hopping wire struct → mapping → domain + struct → runner. + +### Why the split is the right call for MXC + +It earns its keep because of three load-bearing facts: (a) the wire model is +*also* the schema + SDK codegen source, a job that wants a pure JSON-shaped type; +(b) there is genuine wire↔runtime impedance (containment resolution, field +reshaping, defaults, deprecated-spelling aliases) that must live somewhere, and +concentrating it in the parser beats scattering it across backends; (c) the +per-field stability attributes need the wire model as a distinct annotatable +layer. For a config that was a thin pass-through, a single layer would be the +better call — here it is not. + +The real costs (boilerplate, internal drift) are addressable **without merging** +— e.g. a derive/macro for the trivial wire→domain `From` impls, or a compile-time +totality check on the mapping — which captures most of the single-layer ergonomics +while preserving the separation the schema/SDK codegen and stability-attribute work +depend on. No planned phase merges the two models; 2B already reduced three layers +(`Raw*` → … → domain) to two (wire → domain), and a single layer is explicitly not +on the roadmap. + ## Version Negotiation ```