Repository navigation
feat(mxc): add an egress-only AppContainer proxy peer - #4330
pkhodade-NV wants to merge 4 commits into
Conversation
Add an opt-in "proxy-peer" mode to the MXC ProcessContainer driver for sandboxes running with MXC networkProxy, where in-sandbox loopback dials are blocked. - Gateway spawns a per-sandbox AppContainer peer (openshell-mxc-peer) that listens on loopback for the MXC networkProxy / allowedProxyPeer and bridges to the gateway over ACL'd named pipes. - Ingress: forward listener backed by the peer (start_peer_relay) with the same nonce-authenticated contract as the control-channel relay. - Egress: peer tunnels proxy connections to the host egress proxy; tunnelled connections are aliased via ForwardedClients so identity resolution works and the per-sandbox password check is skipped. - Supervisor relay keeps the HTTP(S)_PROXY variables MXC injects when it clears the environment for the launched target. - New driver option pc_proxy_peer_path with validation; README section. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Prashant S Khodade <pkhodade@nvidia.com>
Resolve conflicts with the schema 1.0.0 migration (#4252): drop the peer-specific 0.9.0-alpha schema override (1.0.0 supports allowedProxyPeer), follow the removal of default_configuration_id and the hand-written Default impl, and update the peer config test to the renamed MxcNetwork field. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Prashant S Khodade <pkhodade@nvidia.com>
Signed-off-by: Shailendra Singh <shailendras@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-4330.docs.buildwithfern.com/openshell |
|
/ok to test |
@shailendra-nv, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
|
/ok to test c44279a |
shailendra-nv
left a comment
There was a problem hiding this comment.
Requesting changes for three issues in the new proxy-peer path and its relay integration:
- The unpackaged AppContainer peer has no inbound firewall authorization setup or enforced deployment prerequisite. On profiles that block inbound apps without matching rules, the helper can report ready while permitted egress times out.
- Registry ownership keeps peer processes, listeners, pipe endpoints, and profiles alive after stop, workload completion, and post-peer startup failures. Cleanup currently depends on explicit sandbox deletion.
- Proxy environment inheritance also runs for existing non-peer relay launches, adding ambient proxy values that the explicit launch environment previously excluded and changing workload routing.
Each inline comment identifies the triggering configuration, previous versus proposed behavior, and a suggested fix and regression test.
Reviewed head c44279ab9899c1e407a18bb3cadac738c339831a against base d89e4359b810d3d7205bb7d4a706f3c773a59e1f. I reviewed all 31 changed files and traced configuration validation, MXC 1.0 serialization, profile and helper creation, pipe ACLs and framing, tunnel authentication and original-process attribution, relay environment handling, startup/stop/completion/delete paths, forwarding rejection through the gateway/RPC boundary, Windows build artifacts, examples, qualification contracts, published docs, and related skill updates.
The egress-only design is coherent: the peer stays a byte tunnel while policy evaluation, TLS handling, and credential substitution remain in the host proxy. Dynamic forwarding is rejected before opening a listener, with an actionable FailedPrecondition. The documentation consistently records the remaining private-network ingress exposure.
Validation:
- Ran
cargo test --locked --offline -p openshell-supervisor-relay --all-targets --target aarch64-pc-windows-msvcusing an isolated target directory: 7 unit tests and 10 control-channel integration tests passed. - Reproduced ambient proxy propagation through the compiled ARM64 relay's actual launch protocol with a synthetic value absent from the supplied launch environment.
- Checked the firewall prerequisite against Microsoft's MXC networking contract, which explicitly requires authorization scoped to an unpackaged proxy's AppContainer SID, executable, and port.
- The broader three-package native test attempt waited on another build's shared Cargo cache lock; I stopped only that waiting review command and ran the relay suite in isolation. I did not independently reproduce the peer firewall or full MXC terminal-state cleanup scenarios. Those two findings are based on the source paths and the upstream deployment contract. The PR's reported native MXC smoke remains author-provided evidence.
Please address the three inline findings and add coverage for restrictive firewall deployment, peer cleanup before retained-sandbox deletion, and non-peer launches preserving the explicit environment.
| handle.proxy_addr = SocketAddr::from(([127, 0, 0, 1], ready)); | ||
| let egress_enabled = egress.is_some(); | ||
| if let (Some(first), Some(tunnel)) = (handle.egress_first.take(), egress) { | ||
| let task = tokio::spawn(egress_accept_loop( | ||
| Arc::clone(&handle.descriptor), | ||
| egress_pipe_name(&handle.prefix), | ||
| first, | ||
| tunnel, | ||
| handle.proxy_addr, | ||
| )); |
There was a problem hiding this comment.
Authorize the unpackaged AppContainer listener before accepting readiness.
Before this PR, governed traffic reached the gateway's host proxy. With proxy-peer mode enabled, the workload must first reach this new per-sandbox AppContainer listener. READY proves that the helper bound its port, but neither helper startup nor the driver provisions inbound firewall authorization for the new profile SID and ephemeral port. On a Windows Firewall profile that blocks inbound traffic to apps without a matching allow rule, the sandbox can become ready while otherwise permitted HTTP(S) requests time out.
MXC's proxy deployment contract explicitly says allowedProxyPeer and privateNetworkClientServer do not bypass that firewall policy; an unpackaged AppContainer proxy needs authorization scoped to its SID, executable, and port. Please provide lifecycle-managed authorization, or an enforceable deployment prerequisite with an actionable failure before reporting readiness. Add coverage on a host with the firewall enabled and no existing peer allow rule. The reported successful smoke on one host does not establish that prerequisite on other hosts.
There was a problem hiding this comment.
Implemented firewall authorization before publishing peer readiness in 701efe93. The gateway installs a uniquely owned inbound TCP rule scoped to the helper executable, AppContainer SID, ephemeral port, and local/remote 127.0.0.1. It checks active-profile policy and reports an actionable startup error when rule management is denied. Termination removes the owned rule; preparation/startup failures release the profile and process. The deployment prerequisite and orphan cleanup are documented.
Native COM tests inspect the complete rule scope and verify that a permission-denied installation leaves no rule. The actual AppContainer helper was run on this firewall-enabled host: standard-token startup failed before readiness with 0x80070005, and the test proved its profile was removed by recreating it.
Validation still pending: successful elevated rule installation/removal and pc_peer_https_egress_reads_injected_ca_bundle. A standard-token attempt of the positive HTTPS test was blocked by firewall permission. I am leaving this finding open until that positive test actually executes successfully; the negative-path pass is not connectivity evidence.
| { | ||
| let mut registry = registry.lock().await; | ||
| if let Some(entry) = registry.get_mut(&sandbox_id) { | ||
| entry.peer = Some(Arc::clone(&peer)); | ||
| } |
There was a problem hiding this comment.
Release the peer when its workload reaches a terminal state.
This registry reference keeps PeerHandle alive after the lifecycle task returns. Before this change, stopping a ProcessContainer terminated/reaped its workload and released its host proxy. With peer mode, stop_sandbox still takes the host proxy but leaves entry.peer; monitor_exec also clears only the host proxy on successful and nonzero exits. Startup failures after this registration retain the peer as well. The helper's control pipe therefore remains open, leaving openshell-mxc-peer.exe, its listener, pipe endpoints, and AppContainer profile alive until an explicit delete.
Run a short command, stop a long-running workload, or fail launch after peer startup to trigger this. Keeping completed/stopped/failed sandboxes for inspection then accumulates running helpers. MXC rejects restart, so these peers cannot be reused by restarting the workload. Please take and terminate the peer on confirmed termination and post-spawn failure paths, preserving cleanup ownership across timeout/retry paths. Add a lifecycle regression that checks helper exit and resource release before deleting the retained sandbox.
There was a problem hiding this comment.
Implemented terminal-state cleanup in 701efe93. Confirmed stop, successful/nonzero workload exit, and failure before workload launch now take and explicitly terminate the registered peer before reporting completion. This closes its control pipe even while a forward/other owner retains an Arc, deletes its AppContainer profile, and releases its owned firewall rule. A timed-out delete preserves peer ownership until termination is confirmed and the retry succeeds. Failed launch after workload publication retains the peer until the monitor confirms termination.
Added native-resource regressions: terminal_workload_releases_peer_before_sandbox_deletion, failure_before_workload_launch_releases_peer, launch_failure_retains_peer_until_monitor_confirms_termination, and timed_out_delete_preserves_peer_until_confirmed_retry; the existing stop regression also checks peer cleanup. Fixtures own a real child, secured named pipe, and AppContainer profile. Assertions verify child exit, pipe rebinding, and profile recreation while retaining the sandbox/peer Arc.
All passed in the final affected-package run (1,506 passed, 23 ignored) and the targeted peer/relay rerun (26 passed). MXC Nextest has four mock-workload stdio leak annotations; all four reproduced on the original PR head, and the newly added lifecycle tests have none. The native peer resource assertions passed. Removal of an installed firewall rule still awaits the elevated positive check described in the firewall thread.
| let inherited = inherited_proxy_env(|key| std::env::var(key).ok(), &child_env); | ||
| if !inherited.is_empty() { | ||
| eprintln!( | ||
| "[openshell-supervisor-relay] inheriting {} proxy env var(s) set for this container", | ||
| inherited.len() | ||
| ); | ||
| child_env.extend(inherited); |
There was a problem hiding this comment.
Gate ambient proxy inheritance on proxy-peer mode.
This runs for every relay launch, including the existing IsolationSession backend with egress_proxy = false. That backend's exec_config_json sets inheritDefaultEnv = true, so the relay can receive HTTPS_PROXY from the agent user's default environment even when the sandbox launch request omitted it. Before this PR, the nonempty launch environment went through env_clear().envs(child_env) and the target received only the driver/request environment. Now these lines add ambient proxy values back before clearing the environment, so the same target unexpectedly observes and uses a proxy that was never supplied in its launch request.
A default HTTPS_PROXY plus IsolationSession relay wrapping is enough to change the behavior; proxy-peer mode is not required. Please pass explicit launch metadata identifying MXC proxy-peer injection and inherit these variables only for that mode. Add coverage for a non-peer launch preserving the supplied environment and a peer launch inheriting the MXC endpoint.
Verified through the compiled ARM64 relay at this commit: I set a synthetic HTTPS_PROXY only in the relay process, sent a nonempty launch environment containing SYSTEMROOT and a test marker but no proxy variables, and launched cmd.exe /d /c echo %HTTPS_PROXY%. The target printed the ambient proxy value. This exercises the actual launch protocol without MXC; the IsolationSession applicability follows from the existing inheritDefaultEnv configuration.
There was a problem hiding this comment.
Implemented explicit launch metadata in 701efe93. The driver sets inherit_proxy_env true only for an actual MXC proxy-peer launch. The relay defaults it to false, so curated non-peer/IsolationSession launches do not acquire ambient HTTP(S) proxy settings. Peer launches retain the injected endpoint, explicit child values keep precedence, and ambient NO_PROXY is excluded. Protocol version 5 makes stale v4 relay binaries fail explicitly; staging guidance was updated.
Four added tests drive the compiled relay and inspect a real child's environment: curated_launch_without_peer_metadata_drops_ambient_proxy, non_peer_launch_explicitly_drops_ambient_proxy, peer_launch_preserves_mxc_proxy_without_no_proxy_bypass, and peer_launch_keeps_explicit_proxy_override. All four passed. The full relay suite passed 21 tests (7 unit and 14 blackbox), and the gateway version-mismatch test explicitly rejects v4.
Signed-off-by: Shailendra Singh <shailendras@nvidia.com>
|
Review fixes pushed in 701efe93: scoped firewall authorization before peer readiness, peer cleanup on confirmed terminal states with retry ownership, and explicit relay proxy-inheritance metadata (protocol v5). Added native resource regressions, four compiled-relay environment tests, a native firewall scope/ownership test, and an ignored real peer HTTPS/L7 integration test. Architecture, gateway reference, crate README, and Windows/debug skills are updated. Each review thread has a reply with the corresponding tests. Validation on native Windows ARM64 (separate executions, with overlapping coverage):
The full pre-commit check and the installed commit hook both passed, including workspace/e2e Clippy, Rust format, Python lint/format/proto, SDK lint/format, Markdown, Mermaid, protobuf and license checks. The real MXC token probe printed SKIP because this token lacks The four focused MXC leak annotations were reproduced on original head Reproduction commands from this revision (native MSVC environment; the real tests use mise run --skip-tools pre-commit
mise run --skip-tools test:rust
cargo test --locked --offline -p openshell-driver-mxc -p openshell-supervisor-network -p openshell-supervisor-relay --all-targets --target aarch64-pc-windows-msvc
cargo test --locked --offline -p openshell-driver-mxc --test wxc_exec_real --target aarch64-pc-windows-msvc -- --ignored --skip pc_peer_https_egress_reads_injected_ca_bundle --test-threads=1 --nocaptureTooling limits: this host's ARM64 Biome crashed, so lint/format used the same pinned x64 version under Windows emulation; Python used the existing uv-managed x64 interpreter. SDK code generation used an ignored temporary Buf template that invokes the same plugin through Node because the shipped Unix plugin path does not execute on Windows. The aggregate test task failed in shell parity; suites were run independently. No Python, shell-parity, SDK source or tracked tooling dependency changes were made. Still pending before calling the firewall finding fully validated: successful elevated firewall install/removal and the new real peer HTTPS/L7 test. The standard token was denied with With an elevated token and a staged AppContainer-readable helper: $env:OPENSHELL_MXC_PEER_EXE='C:\path\to\openshell-mxc-peer.exe'
cargo test --locked --offline -p openshell-driver-mxc --test wxc_exec_real --target aarch64-pc-windows-msvc -- --exact pc_peer_https_egress_reads_injected_ca_bundle --ignored --test-threads=1 --nocaptureLogs and the detailed evidence report are retained locally under |
Summary
Adds an opt-in, egress-only proxy-peer mode to the MXC ProcessContainer driver. Each sandbox gets a separate AppContainer peer identity; MXC directs the workload's proxy traffic only to that peer, and the peer tunnels it over an identity-restricted named pipe to OpenShell's per-sandbox host proxy for policy enforcement.
Native MXC 1.0 validation showed that this directionality cannot safely provide dynamic forwarding. MXC admits workload-to-peer proxy traffic but provides no identity-scoped peer-to-workload or gateway-to-workload path. This PR therefore rejects dynamic forwarding in proxy-peer mode instead of weakening isolation. The existing default network mode continues to support forwarding with its documented broader private-network ingress and host-loopback posture.
This supersedes the loopback-only approach in #4228. MXC 1.0 cannot implement that PR's sandbox-local-only TCP guarantee: denying ingress produces WinSock
10013, while enabling the required ingress/host-loopback policies broadens reachability beyond the sandbox-local listener.Related Issue
No issue. This is a safety correction and completion of the existing PR based on native MXC 1.0 validation. It supersedes #4228.
Changes
runtimeConfig.networkProxy,processContainer.network.allowedProxyPeer, denied direct egress, and denied host loopback.FailedPreconditionwith an actionable diagnostic.pc_proxy_peer_path; forwarding examples explicitly leave proxy-peer mode disabled.ingress.default = allowpermits private-network server traffic even though host loopback remains denied.openshell-mxc-peer.exein both Windows release build lanes and qualification evidence.Testing
mise run --jobs 1 pre-commitwith the x64 Biome 2.5.4 binary: pass.cargo test -p openshell-driver-mxc --all-targets: pass (228 executed, 14 real-MXC tests ignored in this generic invocation).mise run --skip-tools windows:check:arm64: pass.mise run --skip-tools windows:build:arm64: pass; gateway, CLI, supervisor relay, proxy peer, and Z3 runtime produced.mise run --skip-tools windows:test:arm64: pass (5,073 passed, 28 skipped).mise run --skip-tools windows:test:unsupported:arm64: pass for all selective feature combinations.mise run --skip-tools windows:test:mxc-real:arm64: pass (14 passed; one service-manager privilege probe self-skipped internally because the unelevated host cannot create services).mise run --skip-tools windows:artifacts: pass.mise run --skip-tools windows:qualify:mxc:gb300:contract: pass.mise run --skip-tools windows:e2e:mxc:ws-agent-mock: pass (4/4).mise run --skip-tools windows:e2e:mxc:openclaw-forward-mock: pass.example.comthrough policy; denied unlistedexample.org; denied direct--noproxybypass; rejected dynamic forwarding with the expected diagnostic.cargo test -p openshell-supervisor-network: pass (1,248 passed, 2 ignored).cargo test -p openshell-supervisor-relay: pass (17 passed).The generic
mise run testandmise run e2eaggregates are not runnable on this Windows host because they invoke Bash/WSL, Unix-domain-socket, executable-bit, and Unix-style Buf plugin paths. The native Windows and MXC-specific replacements above cover the changed code and behavior.Checklist