You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Emit errors and warnings as structured ScriptResponse JSON - #1482
Every backend now reports failure through one contract: a structured ScriptResponse
envelope on stderr, a wire error code that says who can act on the failure, and an exit
code that follows from that code. Previously each executor invented its own prose and its
own exit convention, so a caller could not tell a refused request from an unusable host
without string-matching backend-specific text.
The goal of this PR is not to classify all possible errors.
The wire contract
Code
Meaning
Exit
policy_validation
the request is at fault — a field was refused
1
backend_unavailable
the host is at fault — no tier/provider could be built, so the policy was never judged
-1
backend_error
a runtime fault during an already-accepted run
-1
backend_unavailable is deliberately broader than "fall back a tier": it also covers a
missing sandbox, an absent provider, and other host conditions that no change to the
request can resolve. That is why it is not an exit 1.
Warnings
ScriptResponse gains a warnings field, emitted as one JSON envelope per
line, so advisories (for example the Bubblewrap process.cwd advisory) are machine-readable
instead of free prose. The field is Option<Box<Vec<String>>> to keep ScriptResponse at
120 bytes; going over 128 trips clippy::result_large_err in the release gate, and a size
test locks that in.
The one subtle classification.DispatchError in the process-container backend carries
two genuinely different failures, and collapsing them either way broke a test suite:
warnings is added to the executor's ScriptResponse only; surfacing it in the V1 Rust/.NET/Node SDK result types is out of scope.
Known V1 SDK behavior change: Rust v1::spawn/run now return PolicyValidation (not BackendUnavailable) for unsupported enumeratePaths, and BackendUnavailable (not UnsupportedContainment) for WSLC on Windows builds without wslc.
Callers that fall back on the old codes should update; this is accepted for this PR rather than adding a new versioned SDK surface.
Backends still print their human-readable prose alongside the envelope on stderr. That
double-emission predates this branch and is left as-is.
Future work could include sending errors through a separate channel to differentiate them from sandboxed stdout/stderr.
Exit codes are consistent cross-backend w/ structured ScriptResponse JSON #1336 was the starting point for the exit-code and envelope work, but it was not a
complete implementation for the process-container backend; that backend's dispatcher,
fallback, and capture-denials paths are classified here for the first time.
Contract documented in docs/development/architecture/versioning.md (## Error Contract),
with the per-backend notes updated under docs/backends/.
🔍 Validation
Local, after merging main (Windows 11 build 26700, PSEC 1.1 + base-container tier):
CI: scheduled validation run 38377047465 on ad5b809c
This shows that it fixes the remaining errors related to BaseContainer and AppContainer. The [P8d]/[P8e]hostLoopback failures are fixed by this branch.
The debian13-bubblewrap failure is pre-existing in main, and the ubuntu failure is transient.
Entry-point failures printed a bare line and exited 1, which a caller could
not tell apart from a typed policy rejection, and warnings were written as
unstructured stderr text.
Classify every wxc-exec failure by whose problem it is, emit it as a typed
JSON envelope, and carry warnings on ScriptResponse.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Backends reported failures inconsistently: some flattened everything to a
generic backend error, the Bubblewrap and LXC runners set no failure phase at
all, and WSLC rejections exited -1 while ProcessContainer rejections exited 1.
A caller could not tell a refused request from an unavailable host, and the
PowerShell suites had to match on prose banners to find out.
Classify each failure by whose problem it is and derive both the wire code and
the exit code from that:
the request is at fault Rejected policy_validation exit 1
the host is at fault BackendUnavailable backend_unavailable exit -1
the runtime faulted mid-run other phases backend_error exit -1
Every backend now sets a phase, and `ScriptResponse::failure_exit_code` is the
single place the exit code comes from.
Two consequences are worth calling out. A capability the host's Windows build
cannot represent -- `enumeratePaths`, `hostLoopback='allow'` -- now reaches the
wire as `policy_validation`, because dropping the field runs; it used to be
reported as an unavailable backend, which told the caller to give up on the
host. Conversely, a backend this build did not compile in now reports
`backend_unavailable` from the one-shot path too, matching what the state-aware
path already returned, because only a rebuilt executor can serve it.
Add an optional `warnings` array to `ScriptResponse` so the executors stop
interleaving advisory diagnostics with the error channel. It is boxed to keep
the struct under clippy's `result_large_err` threshold, and omitted from the
wire when a run produced none.
Fixes#612
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The P14b and P14c behavioural assertions gate on the `nativeCaptureAvailable`
probe, which answers whether the Learning Mode API is present. That is not the
same question as whether it is implemented: on a host whose exports resolve but
return E_NOTIMPL, the probe says the provider is usable, both runs then fail
before an output path exists, and P14c reports three failures on empty strings.
The runtime already classifies that outcome correctly -- `is_api_unavailable()`
maps it to `BackendUnavailable`, which reaches stderr as a typed
`backend_unavailable` envelope. Gate on what the run reported rather than on
the probe alone, via a shared `Test-BackendUnavailable`. Keying on the typed
code covers every reason a host cannot provide capture, not just the ones a
probe knows to ask about.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dispatch runs after the policy is accepted, so a failure there is the host
failing to build the tier the policy asked for, not a verdict on the request.
Classifying fallback and capture-provider failures as policy_validation made
wxc-exec log them as unsupported_field_for_backend and exit 1, which the
process-container suites read as a validation rejection of a config they
require to be accepted. Only a SID that could not be derived or ACEs that
could not be applied keep their own codes.
Update the harnesses that encoded the previous exit codes: WSLC and isolation
session rejections now exit 1, and the two process-container dispatch refusals
now exit -1. Match the Bubblewrap cwd advisory piecewise, since the advisory is
carried in a JSON warning envelope that escapes the quotes around the path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tier
Two different failures share DispatchError. A field the selected tier cannot
represent names that field and is documented as a rejection, so collapsing it
into backend_unavailable made hostLoopback=allow and enumeratePaths exit -1,
which the suites read as 'accepted and silently dropped' -- the one outcome
P8d and P8e exist to rule out. A tier that could not be built at all, or a
missing denial-capture provider, stays backend_unavailable: no tier existed to
judge the policy against.
Restore the enumeratePaths harness to exit 1 to match.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Audit finalization raised its cleanup diagnostics as bare stderr prose, so a
caller had to scrape them while every other warning arrived as a JSON
envelope. They now travel on the response and are emitted with the rest.
The finalization failure arm hard-coded exit -1 regardless of how the backend
had already classified the run, which could pair a rejection envelope with an
exit status that contradicts it. Deriving the status from the failure phase
keeps the two in agreement, and the appended message reaches stderr through
the error envelope rather than a separate prose line.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Failure classification remains inconsistent across validation and streaming paths, and the Seatbelt availability check can reject valid default executions.
Elliot (theelliotm)
changed the title
Classify every backend failure by who can act on it
Emit errors and warnings as structured ScriptResponse JSON
Oct 13, 2026
Three surfaces reached the same condition by a different route and reported
it differently.
Shared request validation returned a runtime error, so an empty script was a
rejection through `validate` but a `backend_error` through `spawn`, which is
the path callers actually take. Both shared checks judge the request, so they
are rejections. `enumeratePaths` on a backend that cannot represent it now
also matches how the process-container dispatcher classifies the same field.
The streaming and PTY arms for a backend this build omits returned
`unsupported_containment`, while run-to-completion resolution called the same
build and host `backend_unavailable`. The code no longer depends on which
operation the caller chose. Arms for a backend belonging to another operating
system are untouched.
Seatbelt probed for Apple's sandbox CLI on every launch, but only the `Open`
method shells out to it; `Exec`, the default, applies the profile through
`sandbox_init`. A host without the CLI no longer loses the launch method that
does not need it.
Documents the stderr warning envelope alongside the response array, since the
two shapes differ and only the array was described, and records why a missing
experimental opt-in is a host-side code.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drain retained telemetry warnings before early exits
src/tools/wxc/src/main.rs:1424
This early exit occurs after telemetry::init may have retained a warning (for example, ETW registration failure), but it bypasses the only one-shot warning drain at line 1660. The error envelope is emitted while that warning is silently lost from stderr, despite warnings being documented as emitted on failures too. Drain structured warnings before exiting; the same is needed at the other post-telemetry early exits.
This issue also appears in the following locations of the same file:
I reviewed the 43-file diff from base 8188df6 to head 9b7a541 and confirmed that its normalized contents match gh pr diff 1482. The inline comments below are anchored only on added lines; unchanged supporting files were byte-compared with the base. The strongest gap is that Unix executable tests do not assert the new JSON-code/exit contract. This is a COMMENT review, not a request for changes based on claim-only findings. No local build, platform-dependent integration suite, or end-to-end test was run as part of filing this review.
Findings on existing open threads
#3, live Bubblewrap DNS failures mislabeled as policy refusal: continued in discussion_r4277475899.
#6, state-aware error/status mismatch with the PR's blanket claim: continued in discussion_r4277476083 as claim_mismatch, not an introduced runtime regression.
#13, warning JSON test can miss a malformed line: continued in discussion_r4277475994.
#20, ProcessContainer DACL warnings plus other unstructured wxc-exec advisories despite “Resolves #612”: continued in discussion_r4277476034 as claim_mismatch.
Claim-only and unchanged supporting locations
#15 (claim_mismatch, Medium): new docs/development/architecture/versioning.md:718-719 promises wrong-OS unsupported_containment, but the byte-identical state-aware WSLC/IsolationSession gates still return backend_unavailable. The inline comment targets the new claim, not the old gates.
#16-19 (newly_exposed_by_change, Medium): the changed IsolationSession policy mapping, one-shot parser, Hyperlight policy phase, and Windows WSLC feature-off arm leave docs/development/architecture/backends/isolation-session/oneshot.md, the one-shot paragraph of docs/development/architecture/container-lifecycle.md, the earlier Hyperlight exit row, and docs/backends/wslc/wsl-container-getting-started.md stale. Each comment is anchored to the changed behavior rather than forcing an anchor into an unchanged paragraph.
#22 (claim_mismatch, Low): the PR's all-failures envelope wording includes dry-run failures, but the unchanged handle_dry_run_exit path in wxc, lxc, and mxc_darwin exits before emit_backend_error_envelope; dry-run callers still get a human summary rather than an error JSON line. Either emit the envelope on a failing dry run or limit the claim explicitly. This pre-existing early-exit path is not asserted as a new regression.
Verified pre-existing behavior, not attributed as a new regression
mxc_engine/state_aware.rs is byte-identical between base and head; it already returned backend_unavailable for certain missing-feature paths. The state-aware terminal branches already hard-coded exit 1. Finding #6 addresses only the PR's new universal promise.
The host DNS lookup in bubblewrap/common/proxy_network.rs is byte-identical. Finding #3 concerns the newrejected conversion of its errors at bwrap_runner.rs:472, not the resolver itself.
The older Seatbelt Exec-versus-Open rejection and the streaming feature-off classification have resolved review threads. New comments #11, #15, #19, and #24 address separate residual test, state-aware, and documentation gaps; they are not requests to reopen the fixed behavior.
The reason will be displayed to describe this comment to others. Learn more.
Medium (security) - workload stderr can impersonate MXC's JSON error.
Attribution:newly_exposed_by_change - the new backend_unavailable code is presented as a machine-readable host-failure signal on the same stderr channel the sandboxed program can write.
wxc-exec relays workload stderr before calling emit_backend_error_envelope; that emitter writes nothing for an ordinary nonzero workload exit because error_message is empty (script_runner.rs:104-120). A workload can print a plausible {"error":{"code":"backend_unavailable",...}} and exit nonzero. A CLI consumer parsing these lines cannot establish whether this diagnostic came from MXC or from contained code. This is conditional on a consumer trusting stderr; I did not find an in-repo SDK making fallback decisions from it.
Fix: Put executor-originated results on a host-owned channel the workload cannot write to. Until then, document these shared-stream lines as untrusted and do not recommend switching containment based solely on them.
The reason will be displayed to describe this comment to others. Learn more.
Medium (maintainability) - centralize phase-aware ScriptResponse construction.
Attribution:introduced_by_change - this PR adds the same response/phase assembly here and in lxc_runner.rs:47, appcontainer_runner.rs:95, seatbelt_runner.rs:1104, and isolation_session/common/sandbox.rs:91.
The implementations already differ in whether they copy the diagnostic into standard_err, which affects bare-prose duplication alongside the new JSON envelope. Maintaining one error contract through five near-identical constructors invites drift.
Fix: Put the phase-aware response construction in a small shared ScriptResponse constructor and use it at the backend call sites; preserve intentionally different stderr behavior explicitly.
The reason will be displayed to describe this comment to others. Learn more.
Changes requested
This follow-up changes my verdict from the COMMENT review 5534936733 on the same PR head (9b7a541). The 17 inline comments in that review and four subsequent replies already contain the detailed evidence; I am not opening duplicate threads.
Please address these change-attributable issues before merge:
High - Unix executor contract coverage (discussion_r4284388975, introduced_by_change): no Unix executor suite asserts both the structured error code and its exact process exit status for the new failure contract. Add a real refusal and unavailable-backend subprocess case, plus an envelope-serialization assertion.
Medium - Bubblewrap host DNS misclassified as request refusal (discussion_r4277475899, introduced_by_change): SandboxProxy::resolve can fail on host DNS timeout/thread failure, but the new rejected mapping emits policy_validation/exit 1. Keep static invalid-input errors as policy refusals and classify host lookup failures separately.
Medium - response warnings are missing for in-process callers (discussion_r4284389019, introduced_by_change): the new ScriptResponse.warnings field is not populated by mxc_engine::run even when the logger records advisories. Populate it at the shared run boundary or narrow the published response-level contract.
The state-aware exit mismatch in discussion_r4277476083 predates this PR and is a claim_mismatch, not a merge condition by itself. Other Medium/Low comments remain actionable but are not the basis for this verdict. The PR base and head are unchanged since the earlier review; no local build or host-dependent test suite was run for this follow-up.
Access denied stays a policy rejection; an unreachable mount or I/O error is a host fault.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c677fc2-77c3-43b0-a06f-da11d11447c8
[P2] check_delegation also fails on host I/O errors: filesystem_access.rs:107–109 collapses every access() error except ENOENT/ENOTDIR into denied access. With a single readonly path on a failing mount, object normalization skips inspection, and EIO/ESTALE here now becomes policy_validation/exit 1 even though the unchanged request can succeed after recovery. The changed LXC call at lxc_runner.rs:459–460 has the same problem. Preserve the cause in the shared checker: permission denials should remain rejections, while host I/O failures should return LaunchFailed/backend_error with exit -1. Keep both paths fail-closed and add regression coverage for both error categories.
Preserve host path-resolution failures as backend errors
[P2] Indeterminate includes host failures, not just invalid policy. It is returned when current_dir() fails while anchoring a relative path, when Unix canonicalization fails with EIO/ESTALE, and when Windows resolution returns PathCanonical::Unknown. Mapping all of these to Policy now reports policy_validation/exit 1 for requests that can succeed after the host recovers. Preserve the resolution cause through the path-comparison helpers and map host failures to Setup/backend_error with exit -1, retaining policy rejection for unsupported paths. Both cases must still prevent execution; add regression coverage for the distinction.
Preserve V1 UnsupportedContainment mapping on Windows
[P0] SDK breaking change: on Windows without the wslc feature, Rust v1::spawn and v1::run now return BackendUnavailable instead of the previously documented UnsupportedContainment. Both reach this arm through the shared engine (lib.rs:331–363), so callers that select another backend on UnsupportedContainment no longer take that fallback. Preserve the V1 mapping at the SDK boundary, with a feature-disabled regression test, while applying the new classification to executor output or a new versioned SDK surface.
LXC now supports runtimeConfig.networkProxy, so a bare proxy flag passes
validation; use a non-loopback host, which is still refused.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c677fc2-77c3-43b0-a06f-da11d11447c8
Delegation access checks and Hyperlight deniedPaths overlap checks now
distinguish a user/policy problem (policy_validation) from a host I/O or
resolution failure (backend_error). Both still refuse to launch.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c677fc2-77c3-43b0-a06f-da11d11447c8
Seatbelt profile preparation moved to profile_preparation.rs on main; its path and working-directory errors now classify request faults as policy_validation and host faults as backend_error.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c677fc2-77c3-43b0-a06f-da11d11447c8
Share the path-vs-host I/O classification between bwrap and Seatbelt, and gate the Windows-only hyperlight Ambiguous variant.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c677fc2-77c3-43b0-a06f-da11d11447c8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Needs-AttentionRequires attention or a decision from the MXC maintainers.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Elliot (theelliotm)
changed the title
Gudge (MGudgin)
left a comment
microsoft-github-policy-service
Bot
added
the
Needs-Author-Feedback
📖 Description
Every backend now reports failure through one contract: a structured
ScriptResponseenvelope on stderr, a wire error code that says who can act on the failure, and an exit
code that follows from that code. Previously each executor invented its own prose and its
own exit convention, so a caller could not tell a refused request from an unusable host
without string-matching backend-specific text.
The goal of this PR is not to classify all possible errors.
The wire contract
policy_validation1backend_unavailable-1backend_error-1backend_unavailableis deliberately broader than "fall back a tier": it also covers amissing sandbox, an absent provider, and other host conditions that no change to the
request can resolve. That is why it is not an exit
1.Warnings
ScriptResponsegains awarningsfield, emitted as one JSON envelope perline, so advisories (for example the Bubblewrap
process.cwdadvisory) are machine-readableinstead of free prose. The field is
Option<Box<Vec<String>>>to keepScriptResponseat120 bytes; going over 128 trips
clippy::result_large_errin the release gate, and a sizetest locks that in.
The one subtle classification.
DispatchErrorin the process-container backend carriestwo genuinely different failures, and collapsing them either way broke a test suite:
Fallback(EnumeratePathsUnsupported | IngressUnsupported | DaclFallbackDisabled)policy_validation, exit1Fallback(WriteDacUnavailable | SystemRootUnresolved),CaptureDenialsUnsupported,CaptureDenialsUnavailablebackend_unavailable, exit-1Sid(e)Dacl { .. }backend_error, exit-1Scope notes
warningsis added to the executor'sScriptResponseonly; surfacing it in the V1 Rust/.NET/Node SDK result types is out of scope.v1::spawn/runnow returnPolicyValidation(notBackendUnavailable) for unsupportedenumeratePaths, andBackendUnavailable(notUnsupportedContainment) for WSLC on Windows builds withoutwslc.Callers that fall back on the old codes should update; this is accepted for this PR rather than adding a new versioned SDK surface.
double-emission predates this branch and is left as-is.
🔗 References
Resolves #612
complete implementation for the process-container backend; that backend's dispatcher,
fallback, and capture-denials paths are classified here for the first time.
docs/development/architecture/versioning.md(## Error Contract),with the per-backend notes updated under
docs/backends/.🔍 Validation
Local, after merging
main(Windows 11 build 26700, PSEC 1.1 +base-containertier):cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspacecargo test -p mxc-sdk --lib -- dispatcher::teststests/scripts/run_processcontainer_all_tests.ps1CI: scheduled validation run
38377047465 on
ad5b809cThis shows that it fixes the remaining errors related to BaseContainer and AppContainer. The
[P8d]/[P8e]hostLoopbackfailures are fixed by this branch.The debian13-bubblewrap failure is pre-existing in main, and the ubuntu failure is transient.
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see pull request builds)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow