Skip to content

Enforce Job Object cleanup and resource limits in AppContainer runner#317

Closed
npollock1 wants to merge 6 commits into
microsoft:mainfrom
npollock1:feat/job-object-limits
Closed

Enforce Job Object cleanup and resource limits in AppContainer runner#317
npollock1 wants to merge 6 commits into
microsoft:mainfrom
npollock1:feat/job-object-limits

Conversation

@npollock1

@npollock1 npollock1 commented May 14, 2026

Copy link
Copy Markdown

📖 Description

Adds Windows Job Object enforcement to the AppContainer runner so descendant processes are guaranteed to be terminated when wxc-exec finishes (success, timeout, or failure), and introduces an optional process.resourceLimits section for memory, process-count, CPU-rate, and child-process restrictions.

What changed

  • Schema: New process.resourceLimits object on schemas/dev/mxc-config.schema.0.6.0-dev.json with memoryMb, maxProcesses, cpuRatePercent (0..=100), and allowChildProcesses (default true). All optional; 0 for any cap means no limit.
  • Rust models / parser: ResourceLimits struct on CodexRequest; cpuRatePercent > 100 is rejected at parse time. Permissive defaults preserve behavior for omitted sections.
  • AppContainer runner: Always creates a Job Object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE — even when no caps are configured — so descendants cannot outlive the sandbox. Optional caps applied via JOBOBJECT_EXTENDED_LIMIT_INFORMATION and JOBOBJECT_CPU_RATE_CONTROL_INFORMATION. allowChildProcesses: false adds PROC_THREAD_ATTRIBUTE_CHILD_PROCESS_POLICY. The process is spawned CREATE_SUSPENDED, assigned to the job, then resumed (no race window where children can escape). Timeout cleanup uses TerminateJobObject to kill the entire tree.
  • SDK: ResourceLimits in SandboxPolicy with TypeScript-side validation, propagated into ContainerConfig.process.resourceLimits.
  • Docs: docs/schema.md and sdk/README.md updated.

Notable behavior change

GUI launcher-style apps (e.g. modern Windows 11 Notepad) that rely on a launcher → COM activation → separate UI process pattern will have their UI killed when wxc-exec exits, because the activated UI process becomes a member of the job. This is intentional for the one-shot AppContainer model — long-lived/interactive use cases should use the state-aware sandbox lifecycle.

Known gap (out of scope for this PR)

The BaseContainer FlatBuffer (external/windows-sdk/BaseContainerSpecification.fbs) does not currently expose resource-limit fields. The same Job Object approach can be applied externally to the process handle returned by Experimental_CreateProcessInSandbox in a follow-up; nested job objects are supported on Windows 8+. Documented in docs/schema.md.

🔗 References

Closes #316

🔍 Validation

  • cargo build --workspace --target x86_64-pc-windows-msvc — clean
  • cargo test --workspace --target x86_64-pc-windows-msvc --lib344/344 wxc_common tests pass, including:
    • 5 new parser tests for resourceLimits (omitted, full, partial, zero, out-of-range)
    • 3 new Job Object tests in appcontainer_runner: job_limit_info_uses_request_resource_limits, job_object_kills_orphans_on_drop, job_object_terminates_tree
  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • npm run build && npm test (sdk) — 137/137 tests pass, including new createConfigFromPolicy > resourceLimits coverage (omitted, propagated, invalid CPU, negative caps, non-integers, allowChildProcesses: false)

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

npollock1 and others added 6 commits May 14, 2026 10:50
Introduces process.resourceLimits with memoryMb, maxProcesses,
cpuRatePercent, and allowChildProcesses fields. Adds ResourceLimits
struct to CodexRequest with permissive defaults (no caps, child
processes allowed). cpuRatePercent values >100 are rejected at parse
time. Includes 5 new unit tests covering omitted, full, partial, zero,
and out-of-range inputs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Create an AppContainer job object for every run so kill-on-job-close cleans up descendant processes. Apply configured memory, process, CPU, and child-process restrictions, assign the suspended process before resuming, and terminate the job on timeout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 14, 2026 19:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Windows Job Object enforcement to the AppContainer runner so descendant processes are always cleaned up, and introduces an optional process.resourceLimits configuration section (memory, process count, CPU rate, child-process policy) across the schema, Rust parser/models, AppContainer runner, and TypeScript SDK.

Changes:

  • New ResourceLimits model + parser validation in wxc_common, propagated into CodexRequest.
  • AppContainer runner always creates a Job Object with KILL_ON_JOB_CLOSE, optionally applies memory/process/CPU caps, uses CREATE_SUSPENDED + assign + resume to avoid escape, and uses TerminateJobObject for timeouts.
  • SDK exposes ResourceLimits on SandboxPolicy with validation, schema/docs updated.

Reviewed changes

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

Show a summary per file
File Description
src/wxc_common/src/models.rs Adds ResourceLimits struct and field on CodexRequest.
src/wxc_common/src/config_parser.rs Parses and validates process.resourceLimits JSON.
src/wxc_common/src/appcontainer_runner.rs Creates/assigns Job Object, adds child-process attribute, switches timeout to TerminateJobObject, adds tests.
sdk/src/types.ts Adds ResourceLimits interface and SandboxPolicy.resourceLimits.
sdk/src/sandbox.ts Validates and propagates resourceLimits into ContainerConfig.
sdk/src/index.ts Re-exports ResourceLimits.
sdk/tests/unit/sandbox.test.ts Unit tests for resourceLimits validation/propagation.
schemas/dev/mxc-config.schema.0.6.0-dev.json Adds resourceLimits JSON schema.
docs/schema.md Documents resource limits and backend support matrix.
sdk/README.md Documents resourceLimits field.

Comment on lines +838 to +840
spawn_test_process("cmd.exe /c ping -n 30 127.0.0.1", PROCESS_CREATION_FLAGS(0));

unsafe { AssignProcessToJobObject(job.get(), child.process.get()) }.expect("assign");
Comment on lines +409 to +410
let child_process_policy = PROCESS_CREATION_CHILD_PROCESS_RESTRICTED;
if request.resource_limits.child_processes_blocked() {
Comment thread sdk/src/sandbox.ts
process: {
commandLine: '',
timeout: policy.timeoutMs ?? 0,
...(policy.resourceLimits ? { resourceLimits: policy.resourceLimits } : {}),
@alexsnitkovskiy

Copy link
Copy Markdown
Collaborator

As long as this is only for NON-Tesera scenarios and as long as it does not interfere with the Tesera's future resourcing policy this would be OK. So it should be used perhaps down-level and so on...similar or integrated with work Gudge is doing.

@npollock1

Copy link
Copy Markdown
Author

I'm not attached to this implementation. I just wanted to see if it would actually address the referenced bug and I figured it couldn't hurt to share.

@kanismohammed

Copy link
Copy Markdown
Collaborator

Cached the Pull Request and closing it to clean up the repo for Public release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants