Skip to content

Add SandboxBuilder - #1725

Open
jprendes wants to merge 1 commit into
hyperlight-dev:mainfrom
jprendes:builder
Open

Add SandboxBuilder#1725
jprendes wants to merge 1 commit into
hyperlight-dev:mainfrom
jprendes:builder

Conversation

@jprendes

Copy link
Copy Markdown
Contributor

Description

SandboxBuilder is the entry point for creating a sandbox. It gathers machine configuration, host functions, init data and memory mappings, then builds a MultiUseSandbox from a guest binary on disk, a guest binary in memory, or a snapshot.

It merges the roles of SandboxConfiguration, GuestEnvironment and UninitializedSandbox into a single type, so all three can become implementation details.

Every setting has an in place accessor taking &mut self and a with_* counterpart consuming self for chaining. Machine configuration values are readable through get_*.

map_memory_region is unsafe, matching MultiUseSandbox::map_region. The mapped region must stay valid for the lifetime of the built sandbox.

build_from_snapshot errors when init_data or max_guest_log_level are set, as a snapshot already carries both.

Scope

This PR only adds the new API, it does not deprecate any old API, and does not replace the use of the old API throughout the repo.

Copilot AI lite review requested due to automatic review settings August 12, 2026 11:30
@jprendes jprendes added the area/API Related to the API or public interface label Aug 12, 2026
`SandboxBuilder` is the entry point for creating a sandbox. It gathers machine
configuration, host functions, init data and memory mappings, then builds a
`MultiUseSandbox` from a guest binary on disk, a guest binary in memory, or a
snapshot.

It merges the roles of `SandboxConfiguration`, `GuestEnvironment` and
`UninitializedSandbox` into a single type, so all three can become
implementation details.

Every setting has an in place accessor taking `&mut self` and a `with_*`
counterpart consuming `self` for chaining. Machine configuration values are
readable through `get_*`.

`map_memory_region` is `unsafe`, matching `MultiUseSandbox::map_region`. The
mapped region must stay valid for the lifetime of the built sandbox.

`build_from_snapshot` errors when `init_data` or `max_guest_log_level` are set,
as a snapshot already carries both.

Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>
@jprendes jprendes added kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. kind/refactor For PRs that restructure or remove code without adding new functionality. and removed kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. labels Aug 12, 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.

Pull request overview

This PR introduces a new SandboxBuilder API in hyperlight_host as the primary entry point for configuring and constructing MultiUseSandbox instances from a guest binary (file or bytes) or from a snapshot, consolidating configuration, host-function registration, init data, and memory mappings into one builder type.

Changes:

  • Added sandbox::builder::SandboxBuilder with build_from_file, build_from_bytes, and build_from_snapshot constructors plus with_*/in-place setters.
  • Exposed MultiUseSandbox::builder() and re-exported SandboxBuilder from hyperlight_host.
  • Adjusted host function registry internals to support builder-driven registration.

Reviewed changes

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

Show a summary per file
File Description
src/hyperlight_host/src/sandbox/mod.rs Exposes the new builder module under sandbox.
src/hyperlight_host/src/sandbox/initialized_multi_use.rs Adds MultiUseSandbox::builder() entry point returning a default SandboxBuilder.
src/hyperlight_host/src/sandbox/host_funcs.rs Loosens visibility of FunctionRegistry::functions_map to support builder registration.
src/hyperlight_host/src/sandbox/builder.rs New builder implementation + unit tests for building from file/bytes/snapshot.
src/hyperlight_host/src/lib.rs Re-exports SandboxBuilder from the crate root.

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

}

impl SandboxBuilder {
/// Create a builder with the default configuration and no host functions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

HostPrint is always available, this is the same as the existing behaviour.

@ludfjig ludfjig Aug 14, 2026

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.

true, but maybe we can word it something like ".. and no additional host functions" to avoid confusion?

Comment thread src/hyperlight_host/src/sandbox/builder.rs
Comment thread src/hyperlight_host/src/sandbox/builder.rs
Comment thread src/hyperlight_host/src/sandbox/builder.rs
Comment thread src/hyperlight_host/src/sandbox/host_funcs.rs
@jsturtevant

Copy link
Copy Markdown
Contributor

This PR only adds the new API, it does not deprecate any old API, and does not replace the use of the old API throughout the repo.

I like that this just simplifies the API, and would be an almost drop in for https://github.com/hyperlight-dev/hyperlight-sandbox which I would be able to reduce the code there.

What do propose for plans to depreciate the older apis? I think this would provide a bit more flexibility to do things internally

@andreiltd andreiltd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A big fan of this PR 🚀

@jprendes

Copy link
Copy Markdown
Contributor Author

I should have dropped this link in the PR description.
Here's the document describing the API https://hackmd.io/SdWE6fKSShifX-2_DlY4Fw

At the end of the document I added how we can deprecate the old API

We have 3 deprecation levels, in increasing severity:

  • #[doc(hidden)]: the types disappear from rustdoc and from code completion,
    so new code stops reaching for them. Nothing downstream breaks.
  • #[deprecated]: existing users get a compiler warning pointing at the
    builder, or an error where warnings are denied.
  • pub(crate): the types leave the public API. A breaking change, and the only
    one of the three that guarantees no downstream use remains.

Applying #[doc(hidden)] and #[deprecated] together in one release, then
pub(crate) in the next, is the recommended path. The first release hides the
types from new code and warns existing users, the second removes them from the
public API.

I'd be happy to hear different opinions.

@ludfjig ludfjig 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.

I like this! Only one concern:

  • Looks like pr is missing newly added guest_msrs config.

Some minor things:

  • Would it be possible to somehow allow multiple sandboxes to be created from a builder? Or have builder be clonable? Just thinking about ergonomics if multiple identical sandboxes are to be created (I understand this might not be easy with host functions...)
  • Also bikeshedding if any apis should be renamed (my candiadates are max_guest_log_level, map_file_cow, map_memory_region) but not blocking for this pr...
  • Could you add this builder addition to CHANGELOG.md?
  • Do we want to consider adding the possiblity of having sandboxes without any host functions (including no hostprint?)

Comment on lines +37 to +45
/// Builds a [`Sandbox`].
///
/// Start from [`SandboxBuilder::new`], adjust settings through the `with_*`
/// (consuming, chainable) or bare-named (in place) accessors, then call one of
/// the `build_from_*` methods to create the sandbox from a guest binary on
/// disk, a guest binary in memory, or a [`Snapshot`]. Every setting has a
/// default, so a builder with no adjustments is valid.
#[derive(Default)]
pub struct SandboxBuilder {

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.

Maybe a rustdoc example showing off relevant apis could be a good addition

}

/// Like [`Self::max_guest_log_level`], but consumes and returns `self` for chaining.
pub fn with_max_guest_log_level(mut self, level: LevelFilter) -> Self {

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.

thinking about if we can name this something better...

Comment on lines +90 to +93
let host_funcs = self.host_funcs.into_inner().functions_map;
for (func_name, func_entry) in host_funcs {
func_registry.register_host_function(func_name, func_entry);
}

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.

this will re-register default hostprint host function (which is probably fine...?)

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

Labels

area/API Related to the API or public interface kind/refactor For PRs that restructure or remove code without adding new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants