feat(sdk): add captureDenials to the typed SandboxPolicy

[Carlos Alexandro Becker (caarlos0)]

Note

Held pending #739 at the reviewer's request — not ready to merge yet.
#739 moves schema 0.8+ routing to PSEC, which interacts with this change.
The branch will be kept green and synced with main in the meantime.

📖 Description

mxc_engine::policy::SandboxPolicy — the typed policy surface re-exported by the Rust SDK crate mxc-sdk — had no way to enable Windows denial capture. build_wire_config emitted processContainer with only leastPrivilege, capabilities, and ui, and SandboxRequest.inner is pub(crate), so a Rust SDK caller could not reach processContainer.captureDenials at all. Enabling it required hand-rolling a JSON config and going around the typed API.

Everything downstream already existed:

• wxc_common::wire::CaptureDenials
• the config_parser mapping, including the learningModeLogging / permissiveLearningMode capability injection
• the BaseContainer runner and the ETL → JSON decode path
• CaptureDenialsOutput on the SDK output side (output_metadata())

Only the typed input path was missing. This PR adds it.

New public API in mxc_engine::policy (re-exported as mxc_sdk::policy):

pub struct CaptureDenialsSection {
    pub mode: CaptureDenialsMode,      // Block (default) | Allow
    pub output_path: Option<String>,
}

pub struct SandboxPolicy {
    // ...
    pub capture_denials: Option<CaptureDenialsSection>,
}

Some(..) enables capture and is emitted as processContainer.captureDenials in the #[cfg(target_os = "windows")] branch of build_wire_config. The existing parser then injects the right learning-mode capability (learningModeLogging for Block, permissiveLearningMode for Allow) and the runner surfaces the Allow-mode security warning through warnings().

Limitations / decisions:

• Windows only. On Linux and macOS the section is accepted and silently ignored, because neither Seatbelt nor Bubblewrap has a learning-mode API. It is dropped rather than rejected, matching how other Windows-only knobs are handled in build_wire_config.
• Breaking change. SandboxPolicy is a public struct and existing literals are exhaustive, so every construction site needs the new field. All 18 in-repo sites are updated in this PR; external callers will need capture_denials: None.
• No schema change: the wire format and JSON schema already carry processContainer.captureDenials.

🔗 References

• docs/schema.md — processContainer.captureDenials (pre-existing wire field this exposes)
• src/core/mxc-sdk/README.md — new "Denial capture (Windows)" section added here

🔍 Validation

Automated:

• cargo test -p mxc_engine — 21 passed, 0 failed (baseline was 17; +4 new tests)
• cargo test -p mxc-sdk --doc — passes; the lib.rs example still compiles
• cargo clippy -p mxc_engine -p mxc-sdk --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean

Windows-gated code was cross-checked from a macOS host, since the emission lives in a #[cfg(target_os = "windows")] block:

• cargo check -p mxc_engine --all-targets --target x86_64-pc-windows-msvc — clean
• cargo clippy -p mxc_engine -p mxc-sdk --all-targets --target x86_64-pc-windows-msvc -- -D warnings — clean

New tests:

• capture_denials_section_deserializes_from_camel_case_json / ..._defaults_to_block_without_output_path — public deserialization contract
• emitted_capture_denials_json_matches_the_wire_contract — cross-platform test that pins the emitted {"mode", "outputPath"} object against wxc_common::wire::CaptureDenials. Because that wire type uses deny_unknown_fields, a misspelled key or a wrong mode spelling fails here rather than only on a Windows host.
• capture_denials_reaches_the_container_policy / ..._absent_leaves_the_container_policy_untouched (Windows) — end-to-end through build_request into ExecutionRequest.policy.capture_denials
• capture_denials_is_ignored_off_windows (Linux/macOS) — pins the ignore behavior

Not yet run: the two Windows-gated tests compile and clippy-check under the Windows target but have not executed on a Windows host. A Windows CI run covers this.

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Updated documentation (if applicable)
• Updated Copilot instructions (if build, architecture, or conventions changed)
• If this PR changes Cargo.lock, the dependency-feed-check check passes (see docs/pull-requests.md)

Cargo.lock is unchanged and no dependencies were added.

📋 Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[Copilot]

🟢 Ready to approve

The typed mapping matches the existing wire contract, platform behavior, documentation, and test coverage.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds typed Windows denial-capture configuration to the Rust SDK policy surface.

Changes:

• Adds capture modes, configuration, wire emission, and tests.
• Documents denial capture and output metadata.
• Updates existing policy literals for the new field.

File summaries

File	Description
src/core/mxc_engine/src/policy.rs	Implements the typed API, mapping, and tests.
src/core/mxc_engine/src/dispatch.rs	Updates test policies.
src/core/mxc-sdk/README.md	Documents denial capture usage.
src/core/mxc-sdk/src/lib.rs	Updates the crate example.
src/core/mxc-sdk/tests/sandbox.rs	Updates policy fixtures.
src/core/mxc-sdk/tests/sdk_helpers.rs	Updates helper tests.
src/core/mxc-sdk/tests/streaming.rs	Updates Seatbelt streaming fixture.
src/core/mxc-sdk/tests/streaming_processcontainer.rs	Updates ProcessContainer streaming fixture.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Richie Gomez (richiemsft)]

The wire contract documents captureDenials as incompatible with network.proxy, but the typed SandboxPolicy can now set both and this path emits both without rejecting the combination. Could we validate this conflict during build_request (or before emission) and add a test covering it? Otherwise typed callers can construct a policy the API says is unsupported and only discover the behavior later in the runner.

[Carlos Alexandro Becker (caarlos0)]

Thanks — I dug into this, but I can't find the incompatibility this is premised on, so I'd rather confirm the source than add a rejection that isn't backed by the contract.

What I searched. The only documented mutual exclusion I can find for captureDenials is with the --audit CLI flag, not network.proxy:

• docs/learning-mode/capabilities.md:90 — "It is also mutually exclusive with captureDenials; use captureDenials.mode: "allow" for permissive application-driven capture." — where "It" is --audit.
• That one is already enforced, outside this PR: validate_audit_request (src/core/wxc/src/main.rs:213) rejects --audit + captureDenials, covered by audit_mode_rejects_both_capture_denials_modes.

Beyond that I found no proxy-related constraint in docs/schema.md's captureDenials section, in wire.rs, in the generated dev schema (no allOf/not), or in config_parser.rs.

Evidence the combination is actually supported. In base_container_runner.rs:

• L879 — builtinTestServer proxy launches normally on the capture path; there's no guard against it.
• L1841 — normal teardown calls proxy_coordinator.stop() unconditionally, regardless of capture.
• L1533 — the if capture_denials.is_none() { proxy_coordinator.stop(...) } guard is on the job-object-setup failure path only, and it's ownership transfer rather than a conflict: when capture is active the coordinator is moved into the child (L1586) and stopped during capture finalization instead. Stopping it at L1533 would be premature, not forbidden.

Why I'd rather not add the validation blind. build_request builds the same wire config the JSON path uses, and the shared parser accepts captureDenials + network.proxy today. Rejecting it only in the typed path would make the Rust SDK strictly less capable than an equivalent JSON config — the SDK/parser divergence this crate otherwise works hard to avoid — and would break callers using the cooperative proxy while recording denials.

Happy to implement it if you can point me at the specific line in the wire contract, or confirm the conflict is real but undocumented. If it's real, I think it belongs in config_parser.rs so both entry points enforce it, plus a doc note in docs/schema.md — I'd add the test alongside it. If it was --audit you had in mind, that path is already covered.

Leaving this thread open pending your confirmation rather than resolving it.

[Carlos Alexandro Becker (caarlos0)]

Added tests in 8ed9487 that pin this behaviour, so the question is settled by CI rather than by my reading of the docs.

wire_contract_accepts_capture_denials_together_with_a_network_proxy — drives config_parser::load_request_from_value directly with a config carrying both network.proxy and processContainer.captureDenials, and asserts both survive. That function is the wire contract's authority (it's exactly what build_request delegates to at policy.rs:687), and its captureDenials branch at config_parser.rs:868 is not Windows-gated — so this test runs on every CI platform, not just the Windows jobs. It passes.

capture_denials_and_network_proxy_survive_the_typed_path_together — the Windows-gated end-to-end counterpart through build_request, confirming the typed policy emits both sections and the parser accepts the result unchanged.

Verified locally: cargo test -p mxc_engine → 22 passed (was 21), and cargo clippy -p mxc_engine --all-targets --target x86_64-pc-windows-msvc -- -D warnings clean, so the Windows-gated one compiles too.

So the combination is supported today on both the JSON and typed paths, and adding a rejection would regress it. If the incompatibility is real but undocumented, I'm happy to implement it — but it would need to go in config_parser.rs so both entry points enforce it, plus a note in docs/schema.md, since rejecting it only in the typed path would leave the JSON path accepting a config the SDK refuses.

Still leaving this open for you to confirm rather than resolving it myself.

[Richie Gomez (richiemsft)]

You are correct. I rechecked this against PR #748's actual base rather than the still-unmerged #739 changes I had in context. On #748's main base, neither wire.rs, docs/schema.md, nor config_parser.rs declares or enforces a captureDenials + network.proxy conflict, so my original statement that the wire contract documented one was misleading. The tests in 8ed9487 correctly describe the current contract. I'm retracting this finding and resolving the thread. The separate integration concern is that #739 changes schema 0.8+ routing to PSEC and currently rejects PSEC proxy until peer-identity plumbing exists; that should be handled when these branches are reconciled, not presented as an existing #748 wire-contract violation. Sorry for the confusion.

[Richie Gomez (richiemsft)]

Can you hold this PR until #739 goes in?

[Carlos Alexandro Becker (caarlos0)]

No problem at all, and thanks for digging back into it — much appreciated.

Happy to hold. I won't merge this until #739 is in; I'll leave the PR open and green in the meantime, and re-sync it with main as needed so it stays mergeable.

For whoever reconciles the two: the schema 0.8+ / PSEC routing in #739 is the interesting interaction, since this PR only adds a typed front door onto the existing wire fields. SandboxPolicy.capture_denials maps to the same processContainer.captureDenials object the JSON path already emits, and build_request hands it to config_parser::load_request_from_value like any other section — so if #739 lands a rule that rejects captureDenials with a proxy under PSEC, it should land in config_parser.rs and both entry points will inherit it, with no change needed here beyond updating my two tests to match the new contract.

Ping me when #739 is close and I'll merge main in and re-verify.

[Richie Gomez (richiemsft)]

merged #739, it doesn't reject it has a fallback now but there is a bug that will be followed up by Branden regarding the network proxy policy.

[Copilot]

🟡 Changes recommended

Public documentation omits the native BaseContainer requirement and fallback failure behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/core/mxc_engine/src/policy.rs:539

• This public rustdoc implies denial capture works for the Windows ProcessContainer generally, but dispatcher.rs:342-345 rejects the request whenever the host selects an AppContainer fallback. Document the native BaseContainer/Learning Mode host requirement and the backend_unavailable failure so SDK callers can handle unsupported Windows hosts.

/// Denial-capture section of a [`SandboxPolicy`] (Windows ProcessContainer
/// only). Its presence enables capture: the runner records the sandboxed
/// process's ungranted access attempts and writes a JSON denials document,
/// reported back through
/// [`SandboxOutputMetadata::capture_denials`](wxc_common::models::SandboxOutputMetadata).

src/core/mxc-sdk/README.md:59

• The README omits the key Windows host limitation: dispatcher.rs:342-345 rejects capture when ProcessContainer resolves to either AppContainer fallback tier. Add the native BaseContainer/Learning Mode requirement and resulting backend_unavailable error; otherwise users may treat platform_support reporting ProcessContainer as sufficient.

`SandboxPolicy::capture_denials` enables the Windows ProcessContainer's
learning-mode capture: the runner records every access the policy does not
grant and writes them to a JSON denials document.

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The typed mapping matches the existing wire contract and is covered across supported platform behaviors.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Richie Gomez (richiemsft)]

/azp run

[azure-pipelines]

No pipelines are associated with this pull request.