Repository navigation
bwrap: Add the option to skip the procfs mount - #1508
Open
Wes Turner (westurner) wants to merge 2 commits into
Open
Wes Turner (westurner) wants to merge 2 commits into
Wes Turner (westurner) wants to merge 2 commits into
Conversation
Add additive SDK entrypoints for omitting or explicitly mounting procfs without changing the historical behavior of existing spawn and run APIs. Keep the private PID namespace in no-proc mode and remove the proc mount only from Bubblewrap options before the command separator. Reserve the internal procfs control variable so external process environments cannot select or spoof the SDK policy, and strip the marker before the child environment is constructed. Document the compatibility contract and cover policy mapping, legacy behavior, explicit mount/omit, and malformed Bubblewrap command handling.
Add a cargo-fuzz target for external MXC JSON requests that asserts successful parsing never retains the reserved internal procfs environment marker. Seed ordinary requests and marker values for omission, mounting, and invalid input. The target parses only and never launches a sandbox process. A bounded Linux campaign completed 2,276,062 executions in 31 seconds without a crash.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Policy enforcement, internal-control isolation, policy identity, and fuzz-input handling need correction.
4 open findings
What changed in this PR
Adds optional procfs mounting to Rust V1 Bubblewrap execution, addressing #1507 while preserving existing entrypoint behavior.
Changes:
- Adds explicit procfs policies and execution entrypoints.
- Conditionally mounts
/procand protects the internal environment control. - Adds unit tests, a fuzz target, and migration documentation.
| File | Description |
|---|---|
| src/testing/fuzz/fuzz_targets/procfs_policy.rs | Adds reserved-variable boundary fuzzing. |
| src/testing/fuzz/corpus/procfs_policy/valid.json | Seeds ordinary environment input. |
| src/testing/fuzz/corpus/procfs_policy/unknown-value.json | Seeds an unknown control value. |
| src/testing/fuzz/corpus/procfs_policy/omit.json | Seeds the omit control value. |
| src/testing/fuzz/corpus/procfs_policy/mount.json | Seeds the mount control value. |
| src/testing/fuzz/Cargo.toml | Registers the fuzz target. |
| src/testing/fuzz/.gitignore | Keeps curated JSON seeds tracked. |
| src/mxc-sdk/src/policy.rs | Prepares procfs controls and adds tests. |
| src/mxc-sdk/src/options.rs | Defines ProcfsPolicy. |
| src/mxc-sdk/src/lib.rs | Exposes explicit-policy execution APIs. |
| src/mxc-sdk/src/core/mxc_common/models.rs | Defines the internal control name. |
| src/mxc-sdk/src/core/mxc_common/config_parser.rs | Rejects external reserved-variable input. |
| src/mxc-sdk/src/backends/bubblewrap/common/bwrap_command.rs | Conditionally mounts procfs and strips the control. |
| docs/backends/bwrap/bubblewrap-backend.md | Documents policy choices, migration, and fuzzing. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| prepared.inner.env = request.environment.as_ref().map(|environment| { | ||
| environment | ||
| .iter() | ||
| .filter(|(key, _)| key != crate::mxc_common::models::INTERNAL_MOUNT_PROCFS_ENV) |
Comment on lines
+890
to
+895
| if cfg!(target_os = "linux") | ||
| && matches!( | ||
| request.containment, | ||
| Containment::Process | Containment::Bubblewrap | ||
| ) | ||
| { |
Comment on lines
+903
to
+907
| .get_or_insert_with(Vec::new) | ||
| .push(format!( | ||
| "{}={procfs_control}", | ||
| crate::mxc_common::models::INTERNAL_MOUNT_PROCFS_ENV | ||
| )); |
Comment on lines
+11
to
+22
| use mxc_sdk::mxc_common::config_parser::load_mxc_request; | ||
| use mxc_sdk::mxc_common::logger::{Logger, Mode}; | ||
| use mxc_sdk::mxc_common::state_aware_request::MxcRequest; | ||
|
|
||
| const RESERVED_PROCFS_ENV: &str = "MXC_INTERNAL_MOUNT_PROCFS"; | ||
|
|
||
| fuzz_target!(|data: &[u8]| { | ||
| let Ok(json) = std::str::from_utf8(data) else { | ||
| return; | ||
| }; | ||
| let mut logger = Logger::new(Mode::Buffer); | ||
| if let Ok(MxcRequest::OneShot(request)) = load_mxc_request(json, &mut logger, false) { |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
📖 Description
https://github.com/microsoft/mxc/compare/main...westurner:mxc:add-no_procfs-option?expand=1#diff-7cad66f78ade6321a00b64c32a886643e5dff6354d827879a6c628bf71a4250eR169-R186
🔗 References
🔍 Validation
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see pull request builds)📋 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 pull request builds.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See pull request builds
for the steps.
Microsoft Reviewers: Open in CodeFlow