Add guarded WPR captureDenials fallback - #813
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Forward audit workload exit codes, avoid policy previews from truncated analysis, and retain the legacy current-directory exclusion in the temporary adjusted-config adapter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Store newly added PLM files with repository-standard LF endings so whitespace validation reports the actual content cleanly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Normalize non-verbatim Win32 output aliases during collision checks and keep metadata-only READ_CONTROL denials from producing content-access grants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Compare existing outputs by filesystem identity, fail closed when identity cannot be examined, normalize default-stream aliases, and preserve canonical workload self-access filtering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Use Windows ordinal Unicode case comparison for output collision checks and discard partial capability matches from malformed DACL records. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Compare complete paths when both self-access operands use the same namespace, and only use volume-relative matching for mixed DOS and device paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Inspect missing leaves without following reparse points so dangling links remain unexaminable, and make unknown identity dominate an absent counterpart during collision checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Keep the PLM collision hardening in the Windows resolver without changing Unix filesystem-object semantics or tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Pass the workload exit code using Clap's equals syntax so timeout and error values such as -1 reach plm stop without being parsed as options. Add regression coverage for positive and negative exit codes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Run the public PLM process under the caller token and elevate only authenticated fixed WPR control operations. Transfer ETL data over bounded local pipes, use protected ProgramData scratch storage, and retain a guarded elevated start child for owner-death cleanup and explicit post-stop disarm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Record the asInvoker public helper, restricted self-elevation, authenticated pipe transfer, protected scratch storage, and guarded lifecycle in the repository Copilot instructions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Preserve abandoned mutex ownership, disarm guarded capture immediately after ETL transfer, and normalize case-insensitive verbatim UNC paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Keep live WPR control in-process while wxc-exec owns the singleton, then run public PLM only for existing-trace analysis. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Add a dedicated stopped protocol frame so guarded cleanup is disabled before ETL handling, including failure-safe guardian termination. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Retain one elevated guardian for singleton, WPR start, stop, and cleanup. Fail closed on stale or uncertain recovery without cancelling unverified WPR sessions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Use a post-authentication readiness handshake and PeekNamedPipe polling so empty connected pipes are not mistaken for disconnects. Store default audit artifacts under the caller's local app-data directory so protected installations remain writable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Use protected working directories for elevated PLM and WPR processes, and restore a bounded audit-analysis watchdog with concurrent output draining and process termination. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Fail closed on uncertain WPR teardown, bound control output draining, restore path validation, and document the final recovery model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Track sandbox process lifetimes and add a bounded elevated protocol that returns process-scoped denial analysis without exposing host-wide ETL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Resolved the conflicts with current |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 32 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
docs/learning-mode/capabilities.md:139
- The unchanged validation paragraph immediately below still says build
26657.1002“is rejected,” contradicting this new fallback behavior and the updated OS-support document, which says that build uses legacy containment. Update this block so it distinguishes rejection of native capture from fallback of the overall request.
> **Host selection.** MXC prefers native capture on a feature-enabled Windows
> build exposing the complete official V2 API set:
> `StartLearningModeTrace`, `StopLearningModeTrace`,
> `CloseLearningModeTrace`, `CreateProcessSecurityEnvironment`,
> `QueryProcessSecurityEnvironmentSupport`, and
> `CloseProcessSecurityEnvironment`. When that set is unavailable or cannot
> fully honor the requested policy, MXC retains the highest compatible legacy
> containment tier (SBOX, AppContainer+BFS, or AppContainer+DACL) and pairs it
> with the guarded WPR capture provider. Unsupported hosts return
> `backend_unavailable` only when neither path can preserve the full policy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 32 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/backends/appcontainer/common/src/appcontainer_runner.rs:1825
- Returning here permanently skips
run_teardown().Prepareddoes not clean these policies on drop (NetworkManager::droponly balances Winsock, andFileSystemBfsManagerhas noDrop), so any job-query error or five-second drain timeout leaves the run's firewall/BFS configuration installed. Preserve the enforcement ordering, but after resolving guarded-capture ownership ensure the last job handle is closed/reaped and then execute policy cleanup rather than abandoning it.
if let Err(error) = self.kill() {
capture_output::write_stderr_line_best_effort(format_args!(
"failed to terminate sandbox job during drop: {error}"
));
self.release_guarded_capture_after_termination_failure();
return;
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Summary
I re-resolved this review against base b960cf5 and head 2463a29; the merge-base exactly matches the PR base, and the local/GitHub diffs are line-for-line equivalent apart from abbreviated index hashes.
I am requesting changes primarily because:
- the new asynchronous process attestation can fail the whole sandbox when a descendant exits before the tracker opens it;
- the security-sensitive attach/resume/discard/release failure orchestration has no runtime regression coverage;
- a wedged guardian can block an embedding process for roughly 40 minutes before aborting it; and
- strict job-drain failures now affect non-capture ProcessContainer runs too.
Verified clean, with receipts
- The exact process-lifetime model records creation/exit times, enforces the 4096-generation bound, and has consumer-side PID-reuse tests.
- Manifest changes add only in-tree workspace dependencies and existing Windows API feature gates; there are no new third-party dependencies or unrelated lockfile drift.
- New security-sensitive modules and dependencies are Windows-gated; shared protocol/lifetime types remain portable and non-Windows paths remain fail-closed.
Body-only findings
Medium (documentation) - PLM's new production role is undocumented. README.md and src/host/plm/readme.md are byte-identical between base and head and still describe PLM strictly as an --audit developer workflow. This PR newly makes PLM the automatic production fallback for processContainer.captureDenials, so those existing statements become false because of this change. Please update both overviews.
Medium (testability) - the new job tracker has no deterministic event seam. JobProcessTracker and ProcessTrackerState are new, but directly consume live completion-port and process APIs. Extracting an injectable job-event/handle-resolver seam would allow deterministic testing of delayed notifications, rapid exits, and process-open failures. This is not independently a merge blocker, but it compounds the missing runtime coverage noted inline.
Low (maintainability) - legacy dispatcher wrappers have no production callers. The old dispatch_with_fallback and spawn_with_fallback wrappers now only delegate to the capture-aware entrypoints, and their remaining workspace callers are tests. Consider keeping one run and one spawn entrypoint that accept an optional guarded-capture factory.
Verified pre-existing or withdrawn - not attributed to this PR
Global\Mxc_Plm_Audittest isolation is byte-identical at base and head.run_guarded_stopreplacing an original error when writing the pipe response fails is byte-identical at base and head.- SDK packaging, ESRP signing, and package assertions are explicitly deferred to #834.
- The original performance claims about retained process handles, 64 MB allocation, and polling were withdrawn: handle retention is a deliberate PID-reuse control, 64 MB is a validated cap rather than an eager allocation, and the cited polling was largely pre-existing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/core/mxc_engine/src/guarded_capture.rs:238
- The new fallback is wired into execution, but the public backend probe still derives
CaptureDenialssolely fromBaseContainerRunner::is_capture_denials_usable()(src/core/mxc_engine/src/probe.rs:73-75), which only tests native PSEC/V2. On exactly the legacy hosts enabled by this change,available_backends()therefore reports that capture is unsupported even when the trusted guarded-WPR path is usable. Update the probe to include guarded fallback availability (compatible legacy tier plus a co-located binary passing the trust gate), so SDK consumers do not incorrectly disable the feature.
pub fn factory_for_request(
request: &wxc_common::models::ExecutionRequest,
) -> Option<std::sync::Arc<dyn GuardedCaptureFactory>> {
if request.policy.capture_denials.is_some() {
Some(std::sync::Arc::new(PlmGuardedCaptureFactory))
docs/learning-mode/capabilities.md:144
- This fallback description is contradicted immediately below by the retained statement that build
26657.1002“is rejected.” The updated OS-support document says that build uses legacy containment instead of native capture, so this page now gives conflicting host behavior. Update the validation note to say the earlier native contract routes to guarded legacy capture (when that path is available).
> `CloseProcessSecurityEnvironment`. When that set is unavailable or cannot
> fully honor the requested policy, MXC retains the highest compatible legacy
> containment tier (SBOX, AppContainer+BFS, or AppContainer+DACL) and pairs it
> with the guarded WPR capture provider. Unsupported hosts return
> `backend_unavailable` only when neither path can preserve the full policy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/host/plm/src/elevated.rs:638
- Attestation races bypass the advertised 4096-generation bound: only successfully retained processes are counted at line 653, while every failed attestation increments this counter and can add another entry to
unattested_active. A sandbox that rapidly creates short-lived children can therefore drive unbounded tracker state and continue well past 4096 generations. Count every observedNEW_PROCESSgeneration (including races and the root) toward the limit and trigger the existing terminal tracker-failure path once it is exceeded.
src/backends/appcontainer/common/src/appcontainer_runner.rs:1168 - Permanent guarded-provider failures (missing/untrusted
plm.exe, missing WPR) reach this branch as an ordinaryWxcError::Process, andspawn()later converts that to a default-phaseScriptResponse, so SDK callers receivebackend_error. The updated support contract says an unavailable helper/provider returnsbackend_unavailable. Preserve a typed failure classification from the factory so prerequisite failures map toFailurePhase::BackendUnavailable, while transient failures such as UAC cancellation can remain launch errors.
Err(e) => {
// No active trace exists yet -- terminate the
// still-suspended child now, before it is ever
// resumed, so nothing runs unobserved.
job.terminate_and_wait(u32::MAX)
src/backends/appcontainer/common/src/base_container_runner.rs:2184
- All guarded-session start failures are labeled
LaunchFailedhere, including deterministic host-prerequisite failures such as a missing/untrustedplm.exeor absent WPR. These consequently becomebackend_error, contradicting the documentedbackend_unavailableresult when the guarded helper is unavailable. Carry a typed start error from the factory and map permanent provider/prerequisite failures toFailurePhase::BackendUnavailable, retainingLaunchFailedfor transient cases such as UAC cancellation or singleton contention.
return Err(ScriptResponse {
failure_phase: FailurePhase::LaunchFailed,
..ScriptResponse::error(&message)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/backends/appcontainer/common/src/appcontainer_runner.rs:1168
- Guardian startup failures are flattened to
WxcError::Process, andspawn()then wraps them withScriptResponse::error(FailurePhase::None). Consequently a missing, unsigned, or untrustedplm.exeis exposed bymxc_engine::map_spawn_errorasbackend_error, even though the updated contract says an unavailable guarded helper isbackend_unavailable. Preserve a typedBackendUnavailableresult for helper discovery/trust failures while retaining launch errors for transient startup failures.
Err(e) => {
// No active trace exists yet -- terminate the
// still-suspended child now, before it is ever
// resumed, so nothing runs unobserved.
job.terminate_and_wait(u32::MAX)
src/backends/appcontainer/common/src/base_container_runner.rs:2185
- This classifies every guarded-WPR start failure as
LaunchFailed, including deterministic prerequisite failures such as missing or rejectedplm.exe. Those cases will never succeed on retry and the updated documentation specifiesbackend_unavailable; preserve that phase for helper discovery/signature/trust failures, while reservingLaunchFailedfor transient failures such as UAC cancellation or singleton contention.
return Err(ScriptResponse {
failure_phase: FailurePhase::LaunchFailed,
..ScriptResponse::error(&message)
});
docs/learning-mode/capabilities.md:144
- The immediately following paragraph still says build
26657.1002“is rejected,” contradicting this newly documented fallback and the updated OS-version guide, which says that build uses legacy containment. Clarify that only its earlier native contract is rejected and guarded WPR is selected instead.
> `CloseProcessSecurityEnvironment`. When that set is unavailable or cannot
> fully honor the requested policy, MXC retains the highest compatible legacy
> containment tier (SBOX, AppContainer+BFS, or AppContainer+DACL) and pairs it
> with the guarded WPR capture provider. Unsupported hosts return
> `backend_unavailable` only when neither path can preserve the full policy.
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Verified the review updates through 4a27b4e.
- 14 original findings are addressed.
- The three partially addressed items and remaining Low legacy-wrapper item are accepted as non-blocking.
- The newly identified non-ASCII trust-path normalization issue is also accepted as non-blocking for this approval.
cargo fmt --all -- --checkpassed.- 675 affected crate tests passed with 0 failures and 1 ignored.
📖 Description
Adds a secure
processContainer.captureDenialsfallback for Windows hosts where the complete native PSEC plus V2 Learning Mode capability set cannot honor the requested policy.Native capture remains preferred when PSEC, V2 Learning Mode, and request-specific capabilities are available. Otherwise MXC selects the highest compatible legacy containment tier—SBOX, AppContainer+BFS, or AppContainer+DACL—and pairs it with the guarded WPR capture session. Proxy and least-privilege policies that PSEC cannot represent route to the compatible legacy path rather than weakening policy.
MXC uses a privilege-separated, brokered elevation model. The main executor and public
plm.exeprocess remain unelevated, and a session-scoped UAC-elevated helper is launched only when privileged guarded WPR capture is required. The helper performs a fixed set of WPR control and analysis operations and returns bounded results over an authenticated named pipe.The guarded fallback builds on the PLM architecture refactored in #760 rather than introducing a separate capture or decoding stack. It reuses PLM's
asInvokerfront end, retained elevated guardian, embedded WPR profile, protected scratch and recovery state, authenticated bounded control channel, andlearning_mode_windows::EtlDenialAnalyzercanonical decoder. This PR extends that foundation with sandbox job/process-handle attachment, exact descendant lifetime tracking, and a bounded analysis response that feeds the same canonicalDenialsDocumentoutput as native capture.The guarded path:
DenialsDocumentoutput as nativecaptureDenials.learningModeLoggingand allow/audit collection throughpermissiveLearningMode.TotalProcessesaccounting, with a 4096-generation bound.Runtime resolution locates
plm.exerelative towxc-exec.exeor the loadedmxc_ffimodule rather than the caller's working directory. Before elevation, MXC opens the helper with sharing that denies replacement, resolves the pinned object to a stable local DOS path, verifies Authenticode against the pinned handle, requires the embedded signer organization to be Microsoft Corporation, and validates the owner and DACL of the containing directory and its ancestors. The pinned handle remains open acrossShellExecuteExW, and the resolved path is launched, preventing file replacement and path-alias check/launch races. The elevated child restricts runtime DLL resolution to System32 and fails closed if that hardening cannot be applied.Windows artifact packaging, ESRP signing/verification, and SDK package assertions are defined in #834. Unsigned local helpers and helpers in user-writable locations are intentionally rejected, so guarded-WPR end-to-end execution requires a signed packaged
plm.exein a protected directory. This also applies to Rust SDK source consumers whose module-relative lookup resolves beside a locally compiled executable; native PSEC capture remains available when supported.🔗 References
🔍 Validation
cargo fmt --all -- --checkcargo test -p plm --lib -p mxc_engine— 218 passed, 1 ignored.cargo clippy -p plm -p mxc_engine --all-targets -- -D warningswxc-exec.exeandplm.exe.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type