Wire captureDenials to Windows Learning Mode runtime

[richiemsft]

📖 Description

Wires the merged processContainer.captureDenials contract to the Windows Learning Mode runtime for the BaseContainer tier.

• Maps mode: "block" to learningModeLogging and mode: "allow" to permissiveLearningMode after rejecting user-supplied reserved capability names.
• Enforces one effective mode: allow removes deny-and-record before injecting permissive mode.
• Creates a process security environment, starts capture before launch, attaches the environment with PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT, and launches through normal CreateProcessW.
• Restricts inherited handles with PROC_THREAD_ATTRIBUTE_HANDLE_LIST.
• Supplies an explicit clean user environment to CreateProcessW, preventing
  wxc-exec process variables and secrets from being inherited.
• Uses the caller-provided outputPath or a managed per-run temporary ETL path, then seals the trace after process-tree teardown.
• Rejects unsupported AppContainer fallback directly in the dispatcher before backend construction or DACL mutation.
• Keeps native try_wait() non-blocking; blocking waits and managed Wait/WaitAsync complete process-tree and capture finalization.
• Preserves recovery tracking whenever the security environment may remain live.
• Adds an injectable capture lifecycle seam with deterministic begin/finalization failure tests.

🔗 References

• Inject and validate the two learning-mode capabilities #622
• Consume the Windows Learning Mode trace API: FFI loader + capture lifecycle #661
• Add processContainer.captureDenials config for Windows denial capture #663

🔍 Validation

• cargo test -p learning_mode_windows -p appcontainer_common -- --test-threads=1
• cargo clippy -p learning_mode_windows -p appcontainer_common --all-targets -- -D warnings
• cargo test -p wxc_e2e_tests --no-run
• dotnet build Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj --no-restore
• Feature-enabled V1 Windows VM 172.17.7.153 (build 26657.1002):
  • parent environment sentinel was absent in the sandbox (CLEAN, exit 0)
  • standalone attribute-based capture: exit 0, non-empty ETL
  • block: exit 0, 512-byte ETL
  • allow: exit 0, 3,614-byte ETL, always-visible permissive security warning

✅ 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)

📋 Issue Type

• Bug fix
• Feature
• Task

---

GitHub Actions runs the PR validation build automatically. The ADO pipeline
(MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See docs/pull-requests.md.

If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See docs/pull-requests.md
for the steps.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Connects captureDenials to Windows Learning Mode for BaseContainer execution.

Changes:

• Injects mode-specific Learning Mode capabilities.
• Adds ETL capture lifecycle around BaseContainer processes.
• Rejects unsupported AppContainer fallback.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/core/wxc_common/src/config_parser.rs	Maps capture modes to capabilities and adds tests.
src/Cargo.lock	Records the Learning Mode dependency.
src/backends/appcontainer/common/src/base_container_runner.rs	Implements capture launch and ETL teardown.
src/backends/appcontainer/common/src/appcontainer_runner.rs	Rejects capture on fallback tiers.
src/backends/appcontainer/common/Cargo.toml	Adds the Learning Mode crate dependency.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/backends/appcontainer/common/src/base_container_runner.rs:1089

• Failures from this new call flow into the shared error block, which still reports Experimental_CreateProcessInSandbox failed in both the log and extended_error. That API was not invoked on this branch, so diagnostics point developers at the wrong operation. Track which launch API was used and report CreateProcessAsUserInsideSecurityEnvironment for this path.

            let ok = unsafe {
                launch(
                    HANDLE(ptr::null_mut()), // userToken: caller context
                    ptr::null(),             // applicationName (from command line)
                    cmd_wide.as_mut_ptr(),   // commandLine

src/backends/appcontainer/common/src/base_container_runner.rs:1062

• A host can expose these symbols while the feature is disabled, in which case CaptureSession::begin returns ApiCall with ERROR_CALL_NOT_IMPLEMENTED/E_NOTIMPL. This path hardcodes LaunchFailed, unlike the existing processmodel launch path that classifies those codes as BackendUnavailable, so callers receive the wrong typed failure for an unsupported host. Preserve the API error code and apply the same unavailable classification here.

                    return Err(ScriptResponse {
                        exit_code: -1,
                        error_message: msg.clone(),
                        standard_err: msg,
                        failure_phase: FailurePhase::LaunchFailed,

src/backends/appcontainer/common/src/base_container_runner.rs:681

• This availability check runs only during spawn_base, but ScriptRunner::run skips spawning for dry_run after calling BaseContainerRunner::validate. Consequently, --dry-run reports “validation passed” on a host where BaseContainer exists but these capture exports are unavailable, even though the same request immediately fails at runtime. Include the capture API availability check in backend validation (ideally through a shared helper to avoid drifting checks).

This issue also appears in the following locations of the same file:

• line 1058
• line 1085

        if capture_denials.is_some() {
            let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: captureDenials");
            match (SecurityEnvironmentApi::load(), LearningModeApi::load()) {

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

• This does not preserve mode: "block" after CLI processing. main.rs:1069 later applies --audit by removing learningModeLogging and injecting permissiveLearningMode, while capture_denials.mode remains Block; the capture therefore runs allow-all despite its deny-and-record mode. Reject the --audit + captureDenials combination or synchronize the capture mode during the post-parse CLI mutation.

                CaptureDenialsMode::Block => {
                    policy.capabilities.retain(|capability| {
                        !capability.eq_ignore_ascii_case("permissiveLearningMode")
                    });
                    "learningModeLogging"

[richiemsft]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/backends/appcontainer/common/src/base_container_runner.rs:1096

• This branch launches with CreateProcessAsUserInsideSecurityEnvironment, but the shared failure path at line 1202 still reports Experimental_CreateProcessInSandbox failed. Any capture-specific launch failure is therefore attributed to the wrong API, obscuring diagnosis of the new path. Track which launch API was used and format the failure with its actual name.

            let ok = unsafe {
                launch(
                    HANDLE(ptr::null_mut()), // userToken: caller context
                    ptr::null(),             // applicationName (from command line)
                    cmd_wide.as_mut_ptr(),   // commandLine
                    current_creation_flags,  // creationFlags
                    current_env_ptr,         // environment
                    cwd_ptr,                 // currentDirectory
                    &si,                     // startupInfo
                    session.environment(),   // process security environment
                    &mut pi,                 // processInformation
                )

[Copilot]

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/backends/appcontainer/common/src/base_container_runner.rs:681

• This availability check runs only inside spawn_base, but ScriptRunner::run returns success for dry_run immediately after validate_runner. Since BaseContainerRunner::validate checks only the one-shot BaseContainer API, dry-run incorrectly succeeds on hosts that have BaseContainer but lack these newer capture APIs. Include both Learning Mode API probes in runner validation when capture_denials is set, then reuse or reload them during spawn.

            match (SecurityEnvironmentApi::load(), LearningModeApi::load()) {

src/backends/appcontainer/common/src/base_container_runner.rs:1641

• A trace-seal error returned here becomes the generic non-timeout wait error in sandbox_process.rs:434, which constructs ScriptResponse::error with the default FailurePhase::None. Consequently a post-launch capture failure is serialized and telemetered as having no failure phase. Preserve a typed post-launch error or update the runner mapping so this path reports FailurePhase::PostLaunchFailed.

        let teardown_result = self.run_teardown();
        combine_process_and_teardown_results(result, teardown_result)

src/backends/appcontainer/common/src/base_container_runner.rs:1086

• Failures from this new launch call still enter the shared error block below, which labels the extended error as Experimental_CreateProcessInSandbox failed. For capture runs that API was never called, so diagnostics point developers at the wrong export. Track which launch function was used and report CreateProcessAsUserInsideSecurityEnvironment here.

                launch(

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/backends/appcontainer/common/src/base_container_runner.rs:1516

• A terminal try_wait() does not call this teardown path. Public streaming callers (Sandbox::try_wait and mxc_sandbox_try_wait) can therefore observe that the child exited while the ETL is still unsealed and the trace/security-environment remain live until a later wait() or handle drop. Finalize through the same tree-kill/reap/teardown path when try_wait observes WAIT_OBJECT_0, so completion means the requested output is ready.

        // Seal the learning-mode ETL trace now that the child has exited and
        // been reaped (both `wait` and `Drop` kill + reap before calling this).
        // `finish` stops the trace to the resolved output path, then closes the
        // security environment. The output-file path is surfaced on stderr so a
        // caller can locate the denials (the full NDJSON contract is finalized
        // in the output/consume stage).

src/backends/appcontainer/common/src/base_container_runner.rs:1077

• If this new launch call returns FALSE, the shared failure branch still reports Experimental_CreateProcessInSandbox failed, even though that API was not called. Carry the selected launch API name into the failure path and report CreateProcessAsUserInsideSecurityEnvironment here; otherwise captureDenials failures point operators at the wrong API.

        // The launch yields (api_return_code, last_win32_error_on_failure).
        let (success, last_error) = if let (Some(session), Some(se_api)) =
            (capture_session.as_ref(), capture_se_api.as_ref())
        {
            // Single-attempt in-environment launch. The learning-mode security
            // environment only exists on builds that support the `environment`
            // parameter, so the CreateProcessInSandbox env-not-supported retry
            // does not apply here.
            pi = unsafe { std::mem::zeroed() };

src/backends/appcontainer/common/src/base_container_runner.rs:1065

• This early return occurs after the tracking entry and Ctrl-C cleanup registration are installed, and possibly after the builtin proxy starts, but it does not undo any of them. CaptureSession::begin cleans its own partial environment, but the runner state remains registered/leaked in a long-lived streaming host. Mirror the later launch-failure cleanup before returning.

                Err(e) => {
                    let msg = format!("captureDenials: failed to start learning-mode capture: {e}");
                    let _ = writeln!(logger, "Error: {msg}");
                    return Err(ScriptResponse {
                        exit_code: -1,
                        error_message: msg.clone(),
                        standard_err: msg,
                        failure_phase: FailurePhase::LaunchFailed,
                        ..Default::default()

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/backends/appcontainer/common/src/base_container_runner.rs:1602

• try_wait() now tears down every BaseContainer run, even when captureDenials is not configured. Once the foreground process signals, terminate_and_reap() terminates the job and therefore kills background descendants; the AppContainer implementation still only reports the exit code, and callers may rely on the existing streaming/grace-window behavior. Restrict this eager teardown to capture sessions (or a previously cached capture result) so ordinary BaseContainer polling remains non-destructive.

                self.terminate_and_reap();
                let teardown_result = self.run_teardown();
                combine_process_and_teardown_results(Ok(code as i32), teardown_result).map(Some)

src/backends/appcontainer/common/src/base_container_runner.rs:681

• The required security-environment and trace exports are checked only inside spawn_base(). ScriptRunner::run() returns success for a dry run immediately after BaseContainerRunner::validate(), so --dry-run reports “validation passed” on hosts where captureDenials will always fail as BackendUnavailable. Reuse this API availability check from validate() whenever capture is requested, while keeping the loaded handles for the real spawn path if desired.

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

        if capture_denials.is_some() {
            let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: captureDenials");
            match (SecurityEnvironmentApi::load(), LearningModeApi::load()) {

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/backends/appcontainer/common/src/base_container_runner.rs:1685

• try_wait is documented as a non-blocking observation, but this call invokes kill_process_tree() through terminate_and_reap(). If the root has exited while a background descendant is still running, merely polling with try_wait now terminates that descendant whenever capture is enabled; the same API without capture only reports the root exit. Finalization needs to avoid adding a tree-kill side effect to try_wait (or the API contract must be redesigned explicitly).

                self.terminate_and_reap();

[richiemsft]

Made a few mistakes with the stack tracking:
#709 is the old #699
#710 is the old #701

[richiemsft]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/backends/appcontainer/common/src/base_container_runner.rs:1221

• capture_session is still live here, but this helper classifies the startup-info error and deletes the recovery entry for every non-CleanupFailed error. Only after returning does CaptureSession::Drop attempt the fallible security-environment close, so a close failure leaves broker state untracked. Explicitly abort/finish the session first and use that teardown result to decide whether to remove the entry or mark cleanup deferred.

                    self.cleanup_capture_prelaunch_failure(&error, &request, &sid_string, logger);

src/backends/appcontainer/common/src/base_container_runner.rs:1239

• The PR description says the child is launched through CreateProcessAsUserInsideSecurityEnvironment, but this implementation removes that export and uses CreateProcessW with PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT. Update the description so reviewers and host-requirement documentation reflect the actual API surface.

                CreateProcessW(
                    PCWSTR::null(),
                    Some(PWSTR(cmd_wide.as_mut_ptr())),
                    None,
                    None,
                    !inherited_handles.is_empty(),
                    PROCESS_CREATION_FLAGS(current_creation_flags | EXTENDED_STARTUPINFO_PRESENT.0),

src/backends/appcontainer/common/src/base_container_runner.rs:1305

• For the new security-environment launch, ERROR_NOT_SUPPORTED also means the host feature is unavailable, but the classifier below uses is_api_not_implemented, which excludes it. A CreateProcessW failure with that code is therefore reported as LaunchFailed instead of the documented BackendUnavailable. Include this code only for the capture launch path, or reuse the learning-mode unavailable classifier.

            let extended_error = format!("{launch_api_name} failed: {err:?}");

[MGudgin]

Update the PR description to match the final launch mechanism

The PR description says the child is launched through CreateProcessAsUserInsideSecurityEnvironment, but the final implementation no longer uses that export.

The current code attaches the process security environment using PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT and launches the child with CreateProcessW plus EXTENDED_STARTUPINFO_PRESENT.

Could you update the description so other reviewers and future readers are evaluating the mechanism that will actually merge?

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/backends/appcontainer/common/src/base_container_runner.rs:1156

• This fallible call occurs after the tracking entry and Ctrl-C cleanup registration are installed. The ? returns without cleanup_capture_prelaunch_failure, leaving a stale tracking entry/active registration even though no security environment or child was created. Run the prelaunch cleanup before returning this error.

                    crate::appcontainer_runner::create_default_env_entries().map_err(|error| {

[richiemsft]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).