Skip to content

Add aci_edge_sandboxes with an OpenVMM sandbox backend - #307

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
feat-nvx-rs
Oct 3, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
feat-nvx-rs

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

What

Adds aci_edge_sandboxes/, the aci_edge_sandboxes Rust crate, with
AciEdgeSandbox and AsyncAciEdgeSandbox clients for NVX's state-aware sandbox
lifecycle: provision, start, exec, stop, and deprovision. The default backend
drives OpenVMM directly; no Python tooling or daemon is needed at runtime.

The package-related surfaces consistently use the new name:

  • crate, directory, imports, build metadata, and test helpers:
    aci_edge_sandboxes
  • CI job, action, CLI command, and fake OpenVMM binary:
    aci-edge-sandboxes
  • build, bundle, end-to-end, and required-status environment variables:
    ACI_EDGE_SANDBOXES_*

AciEdgeSandbox and AsyncAciEdgeSandbox remain singular client type names because
each client manages individual sandbox instances. NVX wire identifiers,
sandbox IDs, guest kernel tokens, release layout, and shared artifact-discovery
settings retain their existing names and behavior.

History

The six existing implementation commits were rewritten from their original
merge base. Their commit messages, historical file paths, code, comments, and
documentation use the new package name from its first introduction. A final
formatter-only commit wraps identifiers whose longer names crossed Rustfmt and
Ruff line-length boundaries.

Behavior

  • Extends the managed control agent with cancellation, host mappings, workload
    accounts, feature negotiation, and fail-closed exec-cgroup containment.
  • Exposes synchronous and Tokio lifecycle clients, typed policies and results,
    live streams, cancellation, deterministic validation, and a pluggable
    backend.
  • Drives OpenVMM directly with persistent state, lifecycle locks, process
    reconciliation, authenticated control sessions, filesystem mappings, and
    supported IPv4 egress policies.
  • Provides an in-memory MockBackend and the
    aci-edge-sandboxes-fake-openvmm test double.
  • Wires hosted Linux and Windows crate checks, real-hypervisor lifecycle tests,
    release gating, and required-status aggregation into CI.

Review feedback addressed

  • Containment verification parses a unique valid population field, reports
    read/parse/settlement failures, and refuses cgroup reuse without positive
    emptiness verification.
  • Side-effect-free backend validation hooks reuse the same OpenVMM argument,
    working-directory, timeout, and egress-policy checks as the real operations.
  • Mock input is interruptible on Windows, Unix, and async paths; echo workers
    are closed and joined before terminal outcomes, and input errors remain
    observable.
  • Launch recovery persists child identity before runtime registration and
    bounds cancellation waits by the control timeout.
  • Windows fixtures compare canonical mapped guest paths, including aliased
    temporary-directory spellings.

Artifact pin warning

aci_edge_sandboxes/artifacts.json still pins
v0.1.0-dev.89c192e79f0d, which predates the guest features introduced here.
Until a release containing these changes is available, use
ACI_EDGE_SANDBOXES_BUNDLE_DIR with a current local release directory or point
NVX_ARTIFACTS_DIR at current artifacts.

Validation

Passed on the rewritten source tree:

  • python scripts\nvx.py verify
  • Rust 1.93 formatting, all-target/all-feature Clippy, backend-free Clippy,
    all-feature tests, and default-feature tests
  • synthetic artifact bundling, warning-denied rustdoc, and Rust 1.89
    all-target/all-feature checking
  • the authoritative NVX Python unit-test group and ancillary CLI validation
  • Ruff lint/format, Linux and Windows Pyright, Specula tests, ShellCheck,
    shfmt, and PowerShell syntax parsing
  • exact checks that tracked files, commit subjects, and commit bodies contain
    no stale package, job, command, or environment-variable names

The real-hypervisor lifecycle tests were not rerun locally. The ready PR will
run the KVM, MSHV, and WHP matrices for the rewritten head.

Copilot AI balanced review requested due to automatic review settings October 1, 2026 23:37

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.

Copilot review overview

🟡 Changes recommended

Deterministic validation, cgroup containment verification, and mock cancellation have unresolved correctness or safety defects.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
What changed in this PR

Adds a Rust API for NVX sandbox lifecycle management, backed by OpenVMM, with guest-agent protocol extensions and CI coverage.

Changes:

  • Introduces the nvx Rust crate, synchronous/asynchronous APIs, mock backend, and OpenVMM implementation.
  • Adds guest cancellation, host mappings, workload identity, cgroup containment, and feature negotiation.
  • Integrates crate validation and real-hypervisor lifecycle tests into NVX tooling and CI.
File Description
.dockerignore Excludes Rust build outputs.
.gitignore Ignores Rust-generated files.
.github/​copilot-instructions.md Documents crate validation requirements.
.github/​actions/​check-nvx-rs/​action.yml Defines Rust quality checks.
.github/​actions/​validate-nvx/​action.yml Integrates crate validation.
.github/​workflows/​ci.yml Adds required Rust and hypervisor jobs.
.github/​workflows/​run-nvx-microvm-tests.yml Runs real-hypervisor Rust tests.
README.md Links the Rust API documentation.
doc/​ci.md Documents Rust CI coverage.
doc/​project-structure.md Adds the crate to the repository layout.
doc/​sandbox-design.md Documents guest protocol extensions.
doc/​usage.md Documents test-nvx-rs.
guest/​common/​init Configures workload identities and host mappings.
guest/​common/​nvx-managed-agent.c Adds feature negotiation, cancellation, mappings, and containment.
scripts/​nvx.py Registers the Rust lifecycle command.
scripts/​nvx_tools/​ci.py Makes the Rust job required.
scripts/​nvx_tools/​nvx_rs_tests.py Implements real-hypervisor test invocation.
scripts/​test_nvx_tools.py Tests agent protocol and CLI/CI wiring.
nvx-rs/​Cargo.lock Locks Rust dependencies.
nvx-rs/​Cargo.toml Defines crate metadata, features, and dependencies.
nvx-rs/​README.md Documents API behavior and backend policy.
nvx-rs/​artifacts.json Pins bundled NVX artifacts.
nvx-rs/​build.rs Generates bundled artifact metadata.
nvx-rs/​examples/​lifecycle.rs Demonstrates the sandbox lifecycle.
nvx-rs/​src/​async_api.rs Provides Tokio-compatible wrappers.
nvx-rs/​src/​backend.rs Defines the backend interface.
nvx-rs/​src/​bin/​nvx-fake-openvmm.rs Implements the OpenVMM test double.
nvx-rs/​src/​capabilities.rs Models backend capabilities.
nvx-rs/​src/​cidr.rs Parses and represents CIDR ranges.
nvx-rs/​src/​client.rs Implements the public client facade.
nvx-rs/​src/​error.rs Defines typed lifecycle errors.
nvx-rs/​src/​exec.rs Models execution requests and results.
nvx-rs/​src/​id.rs Implements sandbox identifiers.
nvx-rs/​src/​lib.rs Exposes the crate’s public API.
nvx-rs/​src/​model.rs Defines lifecycle configuration models.
nvx-rs/​src/​openvmm/​artifacts.rs Resolves bundled and external artifacts.
nvx-rs/​src/​openvmm/​config.rs Configures the OpenVMM backend.
nvx-rs/​src/​openvmm/​contract.rs Defines persisted backend contracts.
nvx-rs/​src/​openvmm/​filesystem.rs Plans and validates host mappings.
nvx-rs/​src/​openvmm/​launch.rs Constructs OpenVMM launches.
nvx-rs/​src/​openvmm/​mod.rs Implements OpenVMM lifecycle operations.
nvx-rs/​src/​openvmm/​network.rs Translates egress policies.
nvx-rs/​src/​openvmm/​platform/​linux.rs Implements Linux process handling.
nvx-rs/​src/​openvmm/​platform/​mod.rs Selects platform implementations.
nvx-rs/​src/​openvmm/​platform/​other.rs Handles unsupported platforms.
nvx-rs/​src/​openvmm/​platform/​windows.rs Implements Windows process and pipe handling.
nvx-rs/​src/​openvmm/​process.rs Reconciles OpenVMM processes.
nvx-rs/​src/​openvmm/​protocol.rs Implements the guest control protocol.
nvx-rs/​src/​openvmm/​session.rs Manages authenticated control sessions.
nvx-rs/​src/​openvmm/​state.rs Persists and locks sandbox state.
nvx-rs/​src/​stream.rs Implements execution stream queues.
nvx-rs/​src/​testing.rs Provides the public mock backend.
nvx-rs/​src/​validate.rs Implements generic request validation.
nvx-rs/​tests/​async_api.rs Tests asynchronous wrappers.
nvx-rs/​tests/​mock_backend.rs Tests mock lifecycle behavior.
nvx-rs/​tests/​openvmm_e2e.rs Defines real-hypervisor tests.
nvx-rs/​tests/​openvmm_fake.rs Exercises the backend against fake OpenVMM.
nvx-rs/​tests/​openvmm_unavailable.rs Tests unavailable-artifact behavior.
nvx-rs/​tests/​support/​mod.rs Shares OpenVMM test utilities.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread guest/common/nvx-managed-agent.c
Comment thread nvx-rs/src/client.rs Outdated
Comment thread nvx-rs/src/testing.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 2, 2026 02:59
@ppenna Pedro Henrique Penna (ppenna) changed the title Add the nvx-rs crate with an OpenVMM sandbox backend Add aci_sandbox with an OpenVMM sandbox backend Oct 2, 2026

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.

Copilot review overview

🟡 Changes recommended

CI references a nonexistent dependency ID and does not gate development releases on the new required crate checks.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (3)

Comment thread .github/workflows/ci.yml Outdated

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.

Copilot review overview

🟡 Changes recommended

The CI workflow references an undefined job, while cancellation and interrupted-launch recovery can hang or lose track of a live VM.

Review effort: Balanced
Findings: 3 High severity · 1 Medium severity · 1 Low severity

Open (5)

Comment thread .github/workflows/ci.yml Outdated
Comment thread aci_sandbox/src/openvmm/mod.rs Outdated
Comment thread aci_sandbox/src/openvmm/mod.rs
Comment thread aci_sandbox/src/openvmm/artifacts.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 2, 2026 04:09

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.

Copilot review overview

🔵 Needs a closer look

Network configuration accepts gateway-colliding addresses, and failed E2E cleanup can orphan detached VMs.

Review effort: Balanced
Findings: None

Resolved since last review (5)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Reject guest subnets using reserved host addresses

aci_sandbox/​src/​openvmm/​config.rs:359

guest_network accepts the subnet address, broadcast address, and the first usable address. The portable profile derives that first usable address as its gateway, so a value such as 10.0.0.1/24 passes validation but assigns the guest the gateway's address and breaks networking. Validate the host portion as well, reserving host values 0, 1, and the all-ones value.

Medium severity Preserve state and report VM cleanup failures

aci_sandbox/​tests/​openvmm_e2e.rs:54

Cleanup failures are discarded even though OpenVMM is detached from the test process. If stop cannot terminate the VM, deprovision also fails, then the Python harness removes its temporary state root and loses the PID/capability needed to retry; this can orphan a VM on a self-hosted runner. Preserve the state directory and surface cleanup failures (without double-panicking) whenever stop/deprovision does not complete.

@ppenna
Pedro Henrique Penna (ppenna) marked this pull request as ready for review October 2, 2026 04:18
Copilot AI balanced review requested due to automatic review settings October 2, 2026 04:28
@ppenna

Pedro Henrique Penna (ppenna) commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

PR #307 is ready for review and CI is green on head c1d68ade.

CI run 36964708806, attempt 1, completed successfully: 22 applicable CI jobs passed. This includes Linux/Windows crate gates, OpenVMM unit/vmm tests, real KVM/MSHV/WHP microVM and crate lifecycle tests, all platform benchmarks, the performance regression gate, and the required status check. The license/CLA check passed too. Only development-release publication and performance-history persistence were skipped, as intended for PR runs.

The initial Windows test-fixture path-alias failure was reproduced and fixed in the signed commit above without changing production mapping behavior or weakening assertions. The corrected Windows job and the full replacement run both pass. Local worktrees are clean and the OpenVMM submodule remains unchanged.

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.

Copilot review overview

🔵 Needs a closer look

The security-sensitive guest and process-lifecycle changes require human review and real-hypervisor validation, with two unresolved findings.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity Bundled feature pins an incompatible default release

aci_sandbox/​artifacts.json:3

The default source for the new bundled feature points to a release that predates the guest features required unconditionally by GuestFeatures::REQUIRED. As documented in this PR, a normal --features bundled build using this pin cannot start any sandbox and is rejected as backend_unavailable; users only get a working bundle by supplying an override. Update the pin and all asset digests to a compatible release before exposing this as the feature's default source, or disable the default download path until one exists.

Medium severity UID collision bypasses workload identity compatibility checks

guest/​common/​init:74

Checking only whether the UID exists can skip creation for an incompatible account. resolve_workload_identity immediately requires that UID's primary GID to equal workload_gid and its home to be usable, so a non-root host user whose UID collides with a guest account that has a different GID/home cannot start any sandbox with mapped paths. Verify the existing record is compatible or normalize/create the synthetic workload identity instead of returning solely on UID existence.

Copilot AI balanced review requested due to automatic review settings October 2, 2026 16:50
@ppenna Pedro Henrique Penna (ppenna) changed the title Add aci_sandbox with an OpenVMM sandbox backend Add aci_edge_sandboxes with an OpenVMM sandbox backend Oct 2, 2026

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.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: None

@iyyappam

iyyappam commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

The interfaces for this rust crate needs to be abstract and we need to remove any implementation leaks such as openvmm. #Resolved

Comment thread aci_edge_sandboxes/src/lib.rs Outdated
Comment thread aci_edge_sandboxes/src/lib.rs Outdated

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.

Copilot review overview

🔵 Needs a closer look

Cross-platform process supervision and guest isolation require human review and fresh KVM, MSHV, and WHP validation.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (1)

Comment thread aci_edge_sandboxes/src/lib.rs
Comment thread aci_edge_sandboxes/tests/async_api.rs Outdated
Comment thread aci_edge_sandboxes/tests/mock_backend.rs Outdated

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.

Copilot review overview

🔵 Needs a closer look

Security-sensitive lifecycle changes need human review and real-hypervisor validation, with interrupted-test recovery still unresolved.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (3)

Comment thread scripts/nvx_tools/aci_edge_sandboxes_tests.py Outdated

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.

Copilot review overview

🔵 Needs a closer look

Cross-platform recovery and containment require human review and exact-head KVM, MSHV, and WHP validation.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread aci_edge_sandboxes/src/openvmm/network.rs Outdated

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.

Copilot review overview

🔵 Needs a closer look

Sandbox containment and cross-platform recovery need human review and fresh hypervisor validation, alongside resolution of the validation mismatch.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread aci_edge_sandboxes/src/openvmm/launch.rs
Copilot AI balanced review requested due to automatic review settings October 3, 2026 06:06
Introduce the aci_edge_sandboxes crate with a validated five-operation lifecycle, synchronous and Tokio clients, mock and fake backends, artifact discovery and bundling, and a cross-platform OpenVMM backend.

Extend the managed guest control protocol with feature negotiation, cancellation, host path mappings, dynamic workload identities, and cgroup-based process containment so the backend can enforce its contract.

Wire format, lint, test, and documentation gates plus real-hypervisor lifecycle coverage into the NVX CLI and CI release requirements. Document the API, architecture, policy behavior, and project layout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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.

Copilot review overview

🔵 Needs a closer look

Security-sensitive process supervision and containment require human review and current-head hypervisor validation, alongside the unresolved cleanup issue.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread aci_edge_sandboxes/tests/openvmm_e2e.rs
Copilot AI balanced review requested due to automatic review settings October 3, 2026 06:11

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.

Copilot review overview

🔵 Needs a closer look

Security-sensitive sandbox isolation and cross-platform process recovery need human review and successful KVM, MSHV, and WHP validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

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.

7 participants