Phase 3d: add actionable configuration parse errors

[MGudgin]

This PR adds actionable configuration parse errors: the parser reports
malformed JSON separately from typed policy-shape errors and includes the full
policy path plus whole-file source location in every diagnostic, without leaking
secrets or letting untrusted text corrupt diagnostics or the state-aware stdout
response envelope.

Details

• New config_deserialize module: a path-aware deserialize layer that
  distinguishes JSON syntax errors from typed policy errors, attaching the full
  JSON path and whole-file line/column, escaping control and invisible
  formatting characters (incl. U+2028/U+2029 and bidi overrides), and redacting
  secret-bearing values. Format-character detection uses the
  unicode-general-category crate (Format | LineSeparator | ParagraphSeparator),
  so the Cf/Zl/Zp set tracks the crate's Unicode tables.
• Secret redaction is path-driven: a value is redacted when its policy path
  carries a secret marker. Descriptive markers (token, secret, password,
  ...) match anywhere within a path segment (so wamToken / clientSecret
  redact), while user is matched as a whole path segment so the credential
  bundle (user: { upn, wamToken }) redacts even when a scalar is supplied
  where the object is expected, without colliding with unrelated fields like
  username.
• State-aware per-backend per-phase configs deserialize positionally from the
  retained request text, so typed errors carry whole-file coordinates matching
  base-config errors; navigation uses owned-key maps so escaped sibling keys
  never silently drop the location, with graceful fallback to the value-based
  path.
• Request loading borrows RawValue to discriminate one-shot vs state-aware
  without building a full untyped AST, masking the experimental subtree to
  preserve base-config source locations.
• Diagnostic routing keeps stdout reserved for the state-aware envelope and
  script output; parse and dispatch errors go to auxiliary sinks (log file /
  diagnostic pipe) only and are emitted exactly once. Filesystem-path and
  schema-version diagnostics escape untrusted text via the shared
  escape_diagnostic_text helper.
• Exec dispatch drops the parsed request (and its retained source text) before
  the blocking child run and stdio relay.
• Docs updated: docs/versioning.md, the state-aware API doc, and the
  copilot-instructions config-flow description.

Tests

• Added parser coverage: syntax-vs-typed classification, whole-file line/column
  (computed, not hardcoded), escaped-sibling-key location preservation, exact
  column, CRLF line stability, positional secret redaction, source-present
  locator fallback, malformed-telemetry single auxiliary emission with an empty
  primary buffer, a pinned serde_json positioned-error display-suffix
  contract, format-character escaping (U+202E/U+200B/U+2028/U+2029), and
  segment-aware secret paths (user / nested user / user.upn / [i].user
  redact; username / userProfile do not).
• cargo fmt --all -- --check: passed.
• cargo clippy -p wxc_common -p mxc_engine -p wxc --all-targets -- -D warnings:
  clean.
• cargo test -p wxc_common -p wxc: 538 + 28 passed.
• node scripts/versioning/check-schema-codegen.js,
  check-sdk-types-codegen.js, check-schema-versions.js: passed.

[Copilot]

Pull request overview

Adds a path-aware JSON deserialization layer to produce more actionable configuration parse errors (policy path + source location), while preserving the state-aware stdout envelope contract by routing parse/dispatch diagnostics only to auxiliary sinks.

Changes:

• Introduces config_deserialize to distinguish JSON syntax errors vs typed policy-shape errors, including full JSON paths, source locations, control-character escaping, and secret-value redaction.
• Refactors request loading/discrimination to borrow RawValue for state-aware discrimination (avoids building a full untyped JSON AST) and preserves byte/line locations by masking the experimental subtree.
• Adjusts logger + state-aware driver behavior to keep stdout reserved for envelopes/script output and avoid duplicating state-aware errors into primary stderr/buffer output.

Show a summary per file

File	Description
src/core/wxc/src/main.rs	Logs state-aware dispatch errors only to auxiliary diagnostic sinks before emitting error envelopes.
src/core/wxc_common/src/wire.rs	Improves root-shape error messaging (expecting) and documents optionality expectations for experimental.
src/core/wxc_common/src/state_aware_request.rs	Uses path-aware deserialize for per-backend phase configs and prefixes errors with experimental.<backend>.<phase>.
src/core/wxc_common/src/state_aware_dispatch.rs	Extends tests to assert envelope-ready error paths for typed state-aware backend configs.
src/core/wxc_common/src/logger.rs	Adds log_diagnostic_line to write only to auxiliary sinks (file/diagnostic pipe), and refactors log_line accordingly.
src/core/wxc_common/src/lib.rs	Wires in the new internal config_deserialize module.
src/core/wxc_common/src/config_parser.rs	Refactors request parsing/logging, adds RawValue discriminator + experimental masking for location preservation.
src/core/wxc_common/src/config_deserialize.rs	New: central path-aware deserialize + formatting/redaction/escaping for diagnostics.
src/core/wxc_common/Cargo.toml	Enables serde_json raw_value support and adds serde_path_to_error.
src/Cargo.toml	Adds workspace dependency on serde_path_to_error.
src/Cargo.lock	Locks serde_path_to_error.
docs/versioning.md	Documents the new “actionable parse errors” behavior and guarantees.
docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md	Documents auxiliary-only routing for state-aware parse-phase failures.
.github/copilot-instructions.md	Updates documented config flow to include config_deserialize.

Review details

• Files reviewed: 13/14 changed files
• Comments generated: 2
• Review effort level: Low

[Copilot]

Review details

Comments suppressed due to low confidence (3)

src/core/wxc_common/src/config_parser.rs:190

• This centralized emission is not yet single-shot: convert_wire_config still calls logger.log_line before returning several errors (for example, correlationVector at line 674 and the Seatbelt checks at lines 905/932). The wrapper then logs the returned error again here. One-shot diagnostics are duplicated, and a state-aware Seatbelt validation failure also enters the primary buffer/stderr before being duplicated to the auxiliary sink, violating the new envelope-only routing. Remove the remaining log-before-return calls on error paths and let this wrapper be the sole emitter; warning-only logging can remain.

    if let Err(error) = &result {
        log_error(logger, &error.message(), error.output());
    }

src/core/wxc_common/src/config_parser.rs:251

• The centralized logger receives semantic error strings that are not necessarily escaped. For example, the proxy URL errors at lines 483/489 interpolate the decoded url_str; a JSON \n, bidi override, or U+2028 becomes a real formatting character and is written directly to the console/log file here, allowing forged diagnostic lines. Escape the complete message at this final logging boundary (already-escaped text is unaffected because backslashes are ordinary characters).

fn log_error(logger: &mut Logger, message: &str, output: ErrorOutput) {
    match output {
        ErrorOutput::Primary => logger.log_line(message),
        ErrorOutput::DiagnosticOnly => logger.log_diagnostic_line(message),
    }

src/core/wxc_common/src/config_parser.rs:1258

• This state-aware typed parse still bypasses config_deserialize by using serde_json::from_value. Consequently {"telemetry":{"enabled":"yes"}} reports only invalid experimental.telemetry: ... expected a boolean, with neither the full experimental.telemetry.enabled path nor any whole-file line/column. That contradicts the PR's guarantee for typed configuration diagnostics. Deserialize the retained telemetry source fragment through the path-aware layer and remap its position just like per-phase backend configs.

                // Do not log here: state-aware parse errors are routed centrally
                // and exactly once by the outer `load_mxc_request*` wrapper via
                // `log_error(..., ErrorOutput::DiagnosticOnly)`. Logging here as
                // well would produce a duplicate auxiliary diagnostic.
                // Returning the error keeps stdout clean (envelope-owned) and

• Files reviewed: 14/15 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (3)

src/core/wxc_common/src/config_parser.rs:190

• Central logging is now unconditional, but several convert_wire_config failure paths still call logger.log_line before returning (for example, top-level correlationVector, captureDenials.outputPath, and several proxy guards). A one-shot request hitting any of those paths therefore emits the same error twice, contrary to the exact-once behavior; inner logging can also repopulate primary output before a state-aware error is routed here. Remove the remaining failure-path logger writes and let this wrapper own final error emission.

    if let Err(error) = &result {
        log_error(logger, &error.message(), error.output());
    }

src/core/wxc_common/src/config_parser.rs:1376

• This state-aware telemetry parse bypasses the new path-aware layer by deserializing a cloned Value. For example, experimental.telemetry.enabled: "yes" is reported as invalid experimental.telemetry with no enabled path or whole-file line/column, despite the PR's positional-diagnostic contract. Deserialize the telemetry fragment from the retained source text and remap/prefix its error just like per-phase configuration.

        let telemetry: TelemetryConfig =
            serde_json::from_value(telemetry_val.clone()).map_err(|e| {

src/core/wxc_common/src/config_parser.rs:267

• The escaping hardening remains incomplete for filesystem paths: validate_capture_denials_output_path still interpolates raw path/parent.display() values and logs them. On Unix, an input such as relative\nforged reaches the primary and auxiliary diagnostics with a literal newline, allowing forged log lines despite the new no-control-character guarantee. Apply escape_diagnostic_text to every value formatted by that validator.

        let safe_input = config_deserialize::escape_diagnostic_text(input);

• Files reviewed: 14/15 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (3)

src/core/wxc_common/src/config_parser.rs:130

• Central logging is not yet single-emission for all one-shot failures. convert_wire_config and its validators still call logger.log_line before returning errors (for example, correlationVector at line 728, capability validation at 834/846, and capture-denials path validation at 659-695), so this unconditional call logs the same failure a second time. Remove the remaining fatal-path logging and let this wrapper own emission, while retaining logging only for non-fatal warnings.

    log_one_shot_error(logger, &result);

src/core/wxc_common/src/config_parser.rs:1332

• Malformed state-aware telemetry still bypasses the new path-aware deserializer. For example, an invalid experimental.telemetry.enabled value is rendered only as invalid experimental.telemetry: ..., without the full field path or whole-file line/column; formatting {e} here also bypasses the new Unicode diagnostic sanitization. Deserialize this source fragment through config_deserialize, prefix it with experimental.telemetry, and remap its source position like the per-phase configs.

                // Do not log here: state-aware parse errors are routed centrally
                // and exactly once by the outer `load_mxc_request*` wrapper via
                // `log_error(..., ErrorOutput::DiagnosticOnly)`. Logging here as
                // well would produce a duplicate auxiliary diagnostic.
                // Returning the error keeps stdout clean (envelope-owned) and
                // yields a single auxiliary-sink line.
                WxcError::ConfigParse(format!("invalid experimental.telemetry: {e}"))

src/core/wxc_common/src/config_parser.rs:350

• The filesystem-path hardening is incomplete: validate_capture_denials_output_path still interpolates outputPath and parent.display() directly in its errors (lines 656-695). A relative path containing a newline, ESC, or bidi override therefore still forges diagnostic content, contrary to the shared escaping guarantee. Apply escape_diagnostic_text to every user-derived path used by that validator before formatting.

            let msg = format!(
                "Filesystem path '{}' contains invalid character '\"'",
                config_deserialize::escape_diagnostic_text(path)
            );

• Files reviewed: 14/15 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (4)

src/core/wxc_common/src/config_parser.rs:1325

• This state-aware telemetry conversion bypasses the new path-aware deserializer. For example, experimental.telemetry.enabled: "yes" produces only invalid experimental.telemetry: ..., without the .enabled path or any whole-file line/column, despite the PR's stated guarantee for configuration diagnostics. Locate the borrowed telemetry fragment and deserialize it through config_deserialize, remapping its location as is done for backend phase fragments.

        let telemetry: TelemetryConfig =
            serde_json::from_value(telemetry_val.clone()).map_err(|e| {

src/core/wxc_common/src/config_deserialize.rs:35

• process.env is a common secret-bearing field, but it is not covered by this allowlist. A type error such as "env": "API_KEY=super-secret" is rendered by serde as invalid type: string "API_KEY=super-secret", expected a sequence, so the new diagnostic is copied into logs (and a state-aware error envelope) with the credential intact. Treat the whole env segment as sensitive, just like user.

const SECRET_PATH_SEGMENTS: &[&str] = &["user"];

src/core/wxc_common/src/config_parser.rs:130

• Central logging here does not remove all existing fatal-error logging inside convert_wire_config. Branches such as validate_capture_denials_output_path and the correlationVector/capability/network checks still call logger.log_line before returning the same error, so this wrapper emits those diagnostics a second time (including twice to the file sink). Remove the inline logging from all error-return branches now that the outer wrapper owns fatal emission.

        convert_wire_config(cfg, logger, true, opts.allow_missing_command)
    })();
    log_one_shot_error(logger, &result);

src/core/wxc_common/src/config_deserialize.rs:274

• The claimed shared escaping guarantee is not applied to several manual semantic diagnostics. validate_capture_denials_output_path and capability validation interpolate raw config strings, while convert_wire_proxy echoes the raw URL (including possible userinfo credentials) on host/port errors. Newlines/bidi controls can still forge auxiliary log lines, and proxy credentials can be disclosed. Route every user-derived interpolation through this helper and redact URL userinfo rather than echoing the original URL.

/// Escape control and invisible-format characters in free-form, user-controlled
/// text before it reaches a diagnostic sink. Shared with the manual
/// (non-serde) semantic validators so every user-derived diagnostic honors the
/// same "no raw control/format bytes in diagnostics" guarantee.
pub(crate) fn escape_diagnostic_text(value: &str) -> String {
    escape_control_characters(value)

• Files reviewed: 14/15 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (6)

src/core/wxc/src/main.rs:333

• MxcError messages are not guaranteed to be sanitized. For example, an unknown state-aware sandboxId prefix is interpolated into the error at state_aware_dispatch.rs:183-186; a prefix containing U+2028 or a bidi override reaches this new diagnostic sink unchanged. Escape untrusted control/format characters before writing dispatch errors, as the parse-error path now does.

fn log_state_aware_dispatch_error(logger: &mut Logger, error: &MxcError) {
    logger.log_diagnostic_line(&error.to_string());

src/core/wxc_common/src/config_parser.rs:267

• The new escaping guarantee is not applied to all filesystem/user-derived diagnostics. validate_capture_denials_output_path still interpolates outputPath directly at lines 653-696, and the comma-containing capability error at lines 829-835 likewise embeds arbitrary text. Newlines, U+2028, or bidi controls in those values can still forge diagnostic output; route every user-derived interpolation through the shared escape helper.

        // The file path is untrusted input; on Linux/macOS it may contain
        // newlines or terminal control characters. Escape it before embedding
        // in diagnostics so a missing/unreadable file cannot inject forged
        // multi-line log output.
        let safe_input = config_deserialize::escape_diagnostic_text(input);

src/core/wxc/src/main.rs:328

• State-aware execution returns through run_state_aware_main at main.rs:1005, but the diagnostic pipe is not enabled until main.rs:1125-1128. Consequently this helper can never write state-aware parse or dispatch failures to the diagnostic pipe; without --log-file, the “auxiliary” diagnostic is discarded. Initialize the environment-controlled diagnostic sink before request parsing/state-aware dispatch.

This issue also appears on line 332 of the same file.

/// On a state-aware dispatch failure, record the error only on the auxiliary
/// diagnostic sinks (`--log-file` and the diagnostic pipe) via
/// [`Logger::log_diagnostic_line`]. It is deliberately kept out of the primary

src/core/wxc_common/src/config_parser.rs:250

• Central logging does not yet make errors single-emission because several conversion branches still call logger.log_line before returning an error (for example correlationVector at line 728 and capability validation at lines 834/846). The outer wrappers then call this function and append the same message again, so affected one-shot requests duplicate stderr/buffer and file diagnostics. Make conversion/validation return errors without logging, leaving emission solely to the wrapper.

This issue also appears on line 263 of the same file.

fn log_error(logger: &mut Logger, message: &str, output: ErrorOutput) {
    match output {
        ErrorOutput::Primary => logger.log_line(message),
        ErrorOutput::DiagnosticOnly => logger.log_diagnostic_line(message),
    }

src/core/wxc_common/src/config_parser.rs:1223

• This state-aware policy-shape error is manually constructed, so a request such as a multiline "experimental": 42 gets no line/column even though the source slice is available. That contradicts the new positional typed-error behavior and differs from the one-shot path. Derive this error through the path-aware deserializer or compute its location from the borrowed RawValue.

    if let Some(exp) = experimental_raw.as_ref() {
        if !exp.is_null() && !exp.is_object() {
            return Err(WxcError::ConfigParse(
                "Invalid configuration at `experimental`: expected an object".to_string(),
            ));

src/core/wxc_common/src/config_parser.rs:1325

• Telemetry remains the only typed state-aware config deserialized with raw serde_json::from_value. For experimental.telemetry.enabled: "...", the error lacks the full experimental.telemetry.enabled path and whole-file coordinates, and its attacker-controlled serde text bypasses the new format-character escaping. Use the path-aware positional layer here as well.

        let telemetry: TelemetryConfig =
            serde_json::from_value(telemetry_val.clone()).map_err(|e| {

• Files reviewed: 14/15 changed files
• Comments generated: 0 new
• Review effort level: Medium