Skip to content

Commit fdf1f96

Browse files
GudgeCopilot
andcommitted
Phase 3d: add actionable configuration parse errors
This change reports malformed JSON separately from typed policy errors and makes configuration failures actionable without leaking secrets or allowing untrusted text to corrupt diagnostics. Details: - include complete JSON paths and source locations for one-shot and state-aware failures - preserve state-aware output ownership while rejecting trailing data and malformed experimental blocks - escape control characters, redact secret-bearing values, and avoid retaining a full untyped request tree - document the diagnostic contract and add focused parser coverage Tests: - cargo fmt --all -- --check - cargo check -p wxc_common -p wxc --all-targets - cargo clippy -p wxc_common -p wxc --all-targets -- -D warnings - cargo test -p wxc_common -p wxc - node scripts/versioning/check-schema-codegen.js - node scripts/versioning/check-sdk-types-codegen.js - node scripts/versioning/check-schema-versions.js Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: gpt-5.6-sol Copilot-Session: f66f1e4b-72f7-4508-abb2-3bb3d4c808b9
1 parent fa32478 commit fdf1f96

14 files changed

Lines changed: 1117 additions & 144 deletions

File tree

.github/copilot-instructions.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ The Rust workspace (`src/`) implements multiple sandboxing backends behind the `
118118

119119
### Config flow
120120

121-
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`)
121+
1. User provides JSON config (file or base64) → `config_deserialize.rs` performs path-aware typed deserialization into the wire model (`wxc_common::wire`) → `config_parser.rs` validates and maps it to `ExecutionRequest` (the internal execution model in `models.rs`)
122122
2. `ExecutionRequest` includes the containment backend selection, process config, filesystem/network policies, and optional experimental features
123123
3. The appropriate `ScriptRunner` implementation executes the process and returns `ScriptResponse`
124124

@@ -210,7 +210,7 @@ The workspace is organized into six top-level directories under `src/`:
210210

211211
### Config parser pattern
212212

213-
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.
213+
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). All typed config deserialization goes through `config_deserialize.rs`, which distinguishes syntax errors from typed policy errors and adds the complete JSON path plus source line/column when available; state-aware backend errors are prefixed with their full `experimental.<backend>.<phase>` location. `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.
214214

215215
### TypeScript conventions
216216

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,15 @@ state-aware mode so `stdout` remains parseable without sentinels. (One-shot disp
675675
keeps its existing `stdout` logger behaviour — the stricter routing applies to
676676
state-aware only.)
677677

678+
Configuration parse-phase failures follow the same state-aware contract: the typed
679+
`{error}` envelope is the only primary output, while the human-readable actionable
680+
parse diagnostic is written only to configured auxiliary sinks (`--log-file` and the
681+
Windows diagnostic console). It is not duplicated to the logger's primary
682+
console/buffer output, so a parse failure does not add stderr noise even with
683+
`--debug`. Dispatch-time failures, including typed per-backend configuration
684+
errors, use the same auxiliary-only diagnostic routing before the executor emits
685+
their typed `{error}` envelope.
686+
678687
For exec specifically, MXC diagnostic output mixes with the script's own stderr when
679688
`--debug` is passed. This is a small amount of pre- and post-dispatch noise; consumers
680689
wanting clean separation should use `--log-file <path>` instead, which routes diagnostic
@@ -982,26 +991,30 @@ implements one trait, the other, or both, depending on its declared participatio
982991

983992
### 9.1 Wire envelope (Rust mirror)
984993

985-
MXC's parser at `src/core/wxc_common/src/config_parser.rs` deserializes the
986-
wire-format JSON directly into the typed wire model in
987-
`src/core/wxc_common/src/wire.rs` (`wire::MxcConfig`), then maps it into the
988-
typed domain models (`convert_wire_config``ExecutionRequest`, with `From`
989-
impls beside the domain types for the trivial enum/struct conversions) before
990-
dispatch. The state-aware path reuses this same wire model.
994+
`src/core/wxc_common/src/config_deserialize.rs` performs path-aware JSON
995+
deserialization into the typed wire model in
996+
`src/core/wxc_common/src/wire.rs` (`wire::MxcConfig`).
997+
`src/core/wxc_common/src/config_parser.rs` discriminates request shape,
998+
validates the wire model, and maps it into the typed domain models
999+
(`convert_wire_config``ExecutionRequest`, with `From` impls beside the
1000+
domain types for trivial enum/struct conversions). The state-aware path reuses
1001+
this wire model while retaining its per-backend `experimental` subtree for
1002+
dispatch-time typing.
9911003

9921004
```rust
9931005
// In config_parser.rs — discrimination is by presence of the `phase` key in
994-
// the decoded JSON; both shapes deserialize into the one wire::MxcConfig type,
995-
// which declares `phase` / `sandboxId` and the per-backend `experimental` block.
996-
let value: serde_json::Value = serde_json::from_str(&json_str)?;
997-
if value.get("phase").is_some() {
998-
// state-aware: peel off the raw `experimental` block (typed per-backend at
999-
// dispatch), deserialize the rest into wire::MxcConfig, then map.
1000-
convert_wire_state_aware(value, logger, allow_missing_command)
1006+
// the source JSON without building a full untyped request tree.
1007+
let discriminator: RequestDiscriminator<'_> =
1008+
config_deserialize::from_str(&json_str)?;
1009+
if discriminator.phase.is_some() {
1010+
convert_wire_state_aware(
1011+
&json_str,
1012+
discriminator.experimental,
1013+
logger,
1014+
allow_missing_command,
1015+
)
10011016
} else {
1002-
// one-shot: deserialize wire::MxcConfig from the source text (preserving
1003-
// serde line/column diagnostics) and map.
1004-
let cfg: wire::MxcConfig = serde_json::from_str(&json_str)?;
1017+
let cfg: wire::MxcConfig = config_deserialize::from_str(&json_str)?;
10051018
convert_wire_config(cfg, logger, true, allow_missing_command)
10061019
}
10071020
```

docs/versioning.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,16 @@ HRESULT EnumerateSandboxSpecVersionInfo(
438438

439439
Negotiation failures are **typed and actionable** — never a silent fallback:
440440

441+
- **JSON and policy-shape failures** distinguish malformed JSON syntax from
442+
valid JSON that does not match the typed wire contract. Typed failures name
443+
the full policy path (for example, `network.proxy.localhost`), retain Serde's
444+
expected type/value information, and include source line/column when parsing
445+
directly from request text. State-aware per-backend configuration errors are
446+
prefixed with their full `experimental.<backend>.<phase>` location. Diagnostic
447+
text escapes control characters, and errors at secret-bearing paths redact the
448+
submitted value. After the root JSON value, only whitespace is accepted;
449+
trailing JSON values or other trailing content are rejected as malformed
450+
syntax.
441451
- **Schema-range failures** (Stage 1) carry a clear "older than supported" /
442452
"newer than supported" message telling the caller whether to update the config
443453
or upgrade `wxc-exec`.

src/Cargo.lock

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ windows = { version = "0.62", features = [
9494
windows-core = "0.62"
9595
serde = { version = "1", features = ["derive"] }
9696
serde_json = "1"
97+
serde_path_to_error = "0.1"
9798
thiserror = "2"
9899
anyhow = "1"
99100
base64 = "0.22"

src/core/wxc/src/main.rs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,14 +226,16 @@ fn apply_command_override(
226226
}
227227
}
228228

229-
/// Drives the state-aware dispatch flow. On envelope success, writes the
230-
/// JSON to stdout and exits 0. On exec success, exits with the script's
231-
/// exit code (output already streamed). On failure, writes a JSON error
232-
/// envelope to stdout and exits 1. Diagnostic logger output goes to stderr
233-
/// regardless of mode (per design §7.3 stream protocol — stdout reserved
234-
/// for the response envelope).
229+
fn log_state_aware_dispatch_error(logger: &mut Logger, error: &MxcError) {
230+
logger.log_diagnostic_line(&error.to_string());
231+
}
232+
233+
/// Runs state-aware dispatch while reserving stdout for envelopes or script output.
235234
fn run_state_aware_main(parsed: ParsedStateAwareRequest, dry_run: bool, logger: &mut Logger) -> ! {
236235
let outcome = mxc_engine::run_state_aware(parsed, dry_run);
236+
if let Err(error) = &outcome {
237+
log_state_aware_dispatch_error(logger, error);
238+
}
237239
// Diagnostic buffer flushes to stderr regardless of success/failure so it
238240
// never interleaves with the stdout envelope.
239241
let buffered = logger.get_buffer().to_string();
@@ -1145,6 +1147,31 @@ mod tests {
11451147
Logger::new(Mode::Buffer)
11461148
}
11471149

1150+
#[test]
1151+
fn state_aware_dispatch_errors_use_only_auxiliary_diagnostic_sinks() {
1152+
let directory = tempfile::tempdir().unwrap();
1153+
let log_path = directory.path().join("mxc.log");
1154+
let mut logger = test_logger();
1155+
logger.enable_file_sink(&log_path).unwrap();
1156+
let error = MxcError::malformed_request(
1157+
"Invalid configuration at `experimental.wslc.start.portMappings[0].windowsPort`",
1158+
);
1159+
1160+
log_state_aware_dispatch_error(&mut logger, &error);
1161+
1162+
assert!(
1163+
logger.get_buffer().is_empty(),
1164+
"the JSON envelope must retain primary output ownership"
1165+
);
1166+
drop(logger);
1167+
let log = std::fs::read_to_string(log_path).unwrap();
1168+
assert_eq!(
1169+
log.matches("experimental.wslc.start.portMappings[0].windowsPort")
1170+
.count(),
1171+
1
1172+
);
1173+
}
1174+
11481175
#[test]
11491176
fn cli_parses_flags_after_config_path_without_command_override() {
11501177
let cli = parse_cli(&["wxc-exec", "policy.json", "--experimental", "--debug"]);

src/core/wxc_common/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ schema-gen = ["dep:schemars"]
1212

1313
[dependencies]
1414
serde = { workspace = true }
15-
serde_json = { workspace = true }
15+
serde_json = { workspace = true, features = ["raw_value"] }
16+
serde_path_to_error = { workspace = true }
1617
thiserror = { workspace = true }
1718
base64 = { workspace = true }
1819
url = { workspace = true }

0 commit comments

Comments
 (0)