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
Adds a scenario-first sample suite with parallel Rust, .NET, and Node implementations for:
transient and persisted containers
captured, streaming, and PTY I/O
filesystem and network containment, including blocked-access assertions
ProcessContainer access-denial logging
containment support discovery
telemetry consent and per-run telemetry enablement
The samples use public V1 typed APIs, bounded commands, explicit errors and warnings, and backend-appropriate cleanup. The previous combined .NET sample is replaced by focused scenario projects.
Built the Node SDK and strict type-checked every Node sample
Parsed every sample .csproj and Microsoft.Mxc.Sdk.slnx, and verified all sample projects are included
git diff --check origin/main
Verified the filesystem sample's writable control fails as expected
Full Rust compilation is unavailable in this environment because the MSVC toolchain is not installed. Full .NET compilation is unavailable because the .NET SDK is not installed.
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.
Keep native-owned telemetry string outputs opaque until they are decoded and released with mxc_string_free.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 50ad2d13-4e54-49f2-a50d-0afaddc54611
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Several samples can mask workload or containment failures, the .NET PTY can hang on exit, and clean-checkout Node instructions omit a required SDK build.
The documented Node commands do not work from a clean checkout. Every sample type-checks against ../../../sdk/node/dist, and the installed local package exports dist, but that directory is ignored/not checked in and the SDK only defines prepublishOnly, which npm install here does not build. Update all Node run instructions or automate the prerequisite so the SDK is installed and built before the sample package is installed.
Discarding ExecutionResult makes a timeout or nonzero workload exit look successful and suppresses every warning returned by MXC. Return the workload status and print warnings as the other .NET scenarios do.
This drops the execution result, so timeout/nonzero exit conditions are reported as sample success and run warnings are never shown. Handle the result consistently with the other Node scenarios.
The telemetry run result is discarded, so a timeout or nonzero workload exit still makes this sample return success and all SDK warnings disappear. Preserve the result just as the other scenario samples do so the suite's documented failure/warning behavior remains visible.
Correctly describe missing stdin streaming
samples/README.md:16
The streaming implementations only consume stdout and stderr; none forwards stdin to the spawned process. Describing this scenario as streaming stdin overstates the demonstrated API behavior.
Remove DACL and isolation-tier implementation details from the cross-SDK sample and its documentation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 50ad2d13-4e54-49f2-a50d-0afaddc54611
This table promises stdin streaming, but none of the three implementations reads or forwards stdin; the scenario README correctly describes only stdout and stderr. Narrow the purpose text so users are not directed to this sample for bidirectional streaming.
Build the Node SDK before running sample commands
samples/README.md:26
The documented Node commands do not work from a clean checkout. Every sample resolves types from sdk/node/dist and installs the local SDK package whose exports also point to dist, but that directory is untracked and the SDK has only prepublishOnly (no install/prepare build). The PR validation likewise had to build the Node SDK first. Add a shared prerequisite to install/build sdk/node before installing each sample, or make each sample automate that prerequisite.
Close PTY input and observe the console relay asynchronously so shell exit does not wait for another keystroke.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 50ad2d13-4e54-49f2-a50d-0afaddc54611
Set the Node sample runtime floor to 24.21, remove obsolete IsolationSession build caveats, and use run-to-completion terminology.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 50ad2d13-4e54-49f2-a50d-0afaddc54611
The streaming implementations only consume stdout and stderr; none forwards stdin to the spawned process. Remove stdin from this summary so it does not advertise behavior the sample omits.
The readable control is not enforced: if input.txt is missing or unreadable, cat/type fails but the final success message makes the command exit 0. Chain the message to the read so this scenario actually proves the documented read access.
The readable control is not enforced: if input.txt is missing or unreadable, cat/type fails but the following success message resets the command status to 0. Chain the success message to the read so the sample cannot report success without proving read access.
The readable control is not enforced: if input.txt is missing or unreadable, cat fails but the final printf succeeds, so this sample still exits 0 while claiming the read worked. Chain the success message to cat so a failed read remains a failed workload.
A successful capture artifact may contain zero denials, so this scenario can exit 0 without observing the intended blocked read. Reject a zero count before printing success.
A successful capture artifact may contain zero denials, so this currently exits 0 even when the intended blocked read was not observed. Check totalDenials before reporting success.
A capture report can validly contain zero denials, so merely finding metadata lets this scenario succeed without observing the intended blocked read. Reject total_denials == 0 before reporting success.
Document directory-level readonly grant
samples/README.md:11
This description says one file is granted, but all three implementations put the entire sample directory in readonlyPaths and use it as the working directory. Describe the directory-level grant so readers do not infer narrower containment than the sample applies.
Remove stdin from streaming summary
samples/README.md:16
None of the standard-I/O streaming implementations forwards stdin; they only consume live stdout and stderr. Remove stdin from this summary so it matches the scenario and its implementations.
The non-Windows branch is also used by Linux Bubblewrap, where 127.0.0.1 is private sandbox loopback rather than this host listener (docs/bwrap-support/bubblewrap-backend.md:276-280,752-759). Thus curl fails regardless of whether the requested policy is the cause, and the sample can falsely claim enforcement. Target a backend-reachable host address and validate it with a positive control.
Use a reachable endpoint and establish a positive control
On Linux, process containment uses Bubblewrap, where 127.0.0.1 refers to the sandbox itself rather than this Node server (docs/bwrap-support/bubblewrap-backend.md:276-280,752-759). The request consequently fails even without demonstrating that the deny policy blocked a reachable endpoint, so this can print a false success. Use a backend-reachable host address and establish a positive control first.
Use a reachable host address and verify positive control
On Linux, Containment::Process resolves to Bubblewrap, whose private network namespace makes 127.0.0.1 the sandbox's own loopback, not the host endpoint (docs/bwrap-support/bubblewrap-backend.md:276-280,752-759). This curl therefore fails even if the requested deny policy is not what blocked access, and the sample reports a false containment success. Use a backend-reachable host address and verify a positive control before treating failure as policy enforcement.
Ignoring RunAsync's result makes this sample return 0 for timed-out or nonzero workloads and suppresses all returned warnings. Handle the result like the other .NET run samples.
The execution result is discarded, so SDK warnings, timeouts, and nonzero workload exits are all reported as a successful sample run. Propagate the result consistently with the other Node scenarios.
Discarding run's result hides telemetry warnings and reports success even when the workload times out or exits nonzero. Inspect the result as the other run-to-completion samples do so workload failures remain visible.
Add mocked-native tests for Koffi pointer ownership
sdk/node/src/bindings/telemetry.ts:49
The telemetry tests replace the async binding implementation and do not exercise this ownership-critical Koffi declaration, so the pointer-conversion/freeing regression this change fixes could recur unnoticed. Add a binding-level mocked-native test, analogous to the probe ownership tests, that verifies decoding, freeing the original pointer, and unloading in order.
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
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.
Anis Mohammed Khaja Mohideen (kanismohammed)
left a comment
📖 Description
Adds a scenario-first sample suite with parallel Rust, .NET, and Node implementations for:
The samples use public V1 typed APIs, bounded commands, explicit errors and warnings, and backend-appropriate cleanup. The previous combined .NET sample is replaced by focused scenario projects.
🔗 References
None.
🔍 Validation
cargo fmt --manifest-path samples\Cargo.toml --allcargo generate-lockfile --manifest-path samples\Cargo.tomlcargo metadata --manifest-path samples\Cargo.toml --no-deps --format-version 1.csprojandMicrosoft.Mxc.Sdk.slnx, and verified all sample projects are includedgit diff --check origin/mainFull Rust compilation is unavailable in this environment because the MSVC toolchain is not installed. Full .NET compilation is unavailable because the .NET SDK is not installed.
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
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 GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow