feat: guest command exec over vsock (Hyper.exec/3)#49
Open
markovejnovic wants to merge 39 commits into
Open
Conversation
Implement exec::run/1 and exec::serve_one/2 in src/exec.rs. Replace tests/exec.rs placeholder with three example tests (captures stdout+exit, missing command → 127+stderr, honours cwd). Extend tests/wire.rs request_roundtrips to generate env pairs and assert env survives the round-trip (carry-over from Task 1 review).
Add the `chroot-jail grant-vsock --socket <path>` op and its Elixir wrapper `Hyper.SuidHelper.ChrootJail.grant_vsock/1`. Mirrors `grant-api` exactly: confine the socket path to the jail base via SafePath, walk O_NOFOLLOW from jail_base, require S_IFSOCK + no-follow, chown to caller, chmod 0660, chgrp+chmod 0710 on the parent root dir for traversal. The leaf must end with `.vsock` (vs the fixed `api.socket` name for grant-api). Factors the shared confinement+chown+chmod logic (GrantOut, constants, walk_to, grant_to_caller) into a new `grant.rs` module called by both ops; grant_api.rs re-exports GrantOut for backward compatibility. 14 refusal + pending + grant tests in tests/tools/chroot_jail_grant_vsock.rs (mirrors grant_api's suite, security refusal contracts first).
Implements Hyper.Node.FireVMM.Exec.run/3: connect the firecracker
vsock UDS, complete the CONNECT 1024\n / OK handshake, send the
length-prefixed JSON request frame, and decode the signed-32 exit-code
+ two length-prefixed stdio frames. recv_exactly loops defensively until
all bytes arrive; recv_line accumulates byte-by-byte for the handshake.
Retries on :econnrefused/:enoent/non-OK until the connect_timeout (5 s
default) elapses, then returns {:error, :agent_unavailable}.
Tests: full round-trip against a fake AF_UNIX server + a chunked-response
variant that sends the response one byte at a time to exercise frame
accumulation.
The `.ends_with(".vsock")` suffix check allowed any leaf ending with
`.vsock` (e.g. `evil.vsock`) and rejected the real socket name
`vsock.sock`, causing every boot to loop until the deadline.
Replace the suffix check with an exact-name guard mirroring grant-api's
`SOCKET_NAME` pattern: define `VSOCK_NAME = "vsock.sock"` and require
`leaf == Path::new(VSOCK_NAME)`. Update all tests to use the real
`vsock.sock` leaf so the name gate isn't short-circuited when testing
other refusal contracts; add `wrong_leaf_name_is_rejected` and
`api_socket_name_is_rejected` to pin the exact-name contract.
… client
The Elixir client sends env as a JSON object (`{"K":"V"}`); serde was
trying to deserialize it into `Vec<(String,String)>` and failing with
"invalid type: map, expected a sequence", causing every exec with :env
to return {:error, :closed}.
Change the env field type to `BTreeMap<String,String>`, which serde
deserializes correctly from a JSON object. Update exec.rs to use
`.envs(&req.env)`, and add a cross-language fixture test that feeds the
exact byte sequence the Elixir client produces and asserts correct decoding.
Also fix M1: the signal-exit comment claimed "128+signal" but the code
returns -1; update the comment to match. Fix M3: the Hyper.exec/3 @doc
now names all three accepted timeout opts (:timeout, :timeout_ms,
:connect_timeout) accurately.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Test Results134 tests - 243 134 ✅ - 241 2s ⏱️ -4s Results for commit af89380. ± Comparison against base commit c3a9766. This pull request removes 260 and adds 17 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…he app Replace the manual `mix guest_agent.install` task with a `:guest_agent_build` Mix compiler (mirroring the codegen compilers) that builds the static musl binary into priv/guest-agent/ at compile time, so the agent ships inside the app and its release with no install step. Host arch is required; a cross arch is best-effort (built only when its rustup target is installed, warn+skip on link failure). Provider resolves from :code.priv_dir; ensure_installed/0 now checks the host arch only (KVM boots same-arch guests). Drops the now-unused guest_agent_install_dir.
Contributor
Author
|
Follow-up (per review feedback): the guest agent no longer needs a manual |
Replace length-prefixed JSON framing with self-delimiting CBOR in both
directions. Request: client CBOR.encodes a map and sends raw bytes;
agent reads one value via ciborium::from_reader (no EOF needed).
Response: agent ciborium::into_writer's then closes; client reads to EOF
and CBOR.decodes the accumulated buffer. stdout/stderr are CBOR byte
strings (serde_bytes on Rust, %CBOR.Tag{tag: :bytes} unwrap on Elixir).
Removes serde_json from the guest-agent crate. Cross-language contract
is pinned by a Rust fixture (decodes the exact Elixir-produced anchor
bytes) and a matching Elixir assertion on the same hex anchor.
…unwrap_bytes
Add a shared response anchor (exit_code=3, stdout=[0xFF,0x00,0x68,0x69], stderr=[])
that is pinned on both sides:
- Rust: rust_encodes_response_anchor asserts ciborium emits the exact 32 literal
bytes (byte string, major type 2 for stdout/stderr), catching any regression
where serde_bytes is dropped from wire::Response.
- Elixir: two new tests decode the same hex and round-trip through the fake
server, asserting unwrapped stdout == <<0xFF,0x00,0x68,0x69>>.
The 0xFF,0x00 prefix is invalid UTF-8: a text-string encoder would corrupt or
reject it, making both per-side suites fail on the same regression the anchor
is designed to catch.
Fix 2: unwrap_bytes/1 now returns {:ok, binary()} | {:error, term()} instead of
raising FunctionClauseError on unexpected input shapes. recv_response/2 threads
the result through `with`, so a malformed agent response (e.g. stdout as an
integer list) becomes {:error, {:bad_response, {:not_bytes, other}}} rather than
an unhandled raise that violates the @SPEC.
New test: malformed-response test asserts {:error, _} via the fake server seam.
Contributor
Author
|
Wire protocol reworked → CBOR (replaces the length-prefixed JSON framing described above). After review feedback that the hand-rolled |
… + grpc.gen) - Add proto/hyper/agent/v1/agent.proto: GuestAgent service with Exec and Health RPCs; package hyper.agent.v1. - Add native/guest-agent/build.rs: calls tonic_prost_build::configure() .build_client(false).compile_protos(...) to generate server-side Rust stubs into OUT_DIR/hyper.agent.v1.rs. - Add tonic 0.14.6, prost 0.13.5, tokio 1, tokio-vsock 0.7.2 (tonic014 feature), and tonic-prost-build 0.14.6 to native/guest-agent/Cargo.toml. Note: tonic-prost-build (build dep) pulls prost 0.14 for codegen; the runtime prost 0.13 coexists since the generated module is not yet included (later task). - Refactor Mix.Tasks.Compile.GrpcGen to glob proto/**/*.proto and compile each proto separately with its own directory as --proto_path (required so protoc-gen-elixir derives a bare filename, not a package-doubled path). A second --proto_path=proto enables future cross-proto imports. - Gitignore lib/hyper/agent/v1/ alongside the existing grpc/v0 entry.
…elayed UDS Wires up a minimal tonic Health server (examples/health_uds.rs) that serves over a Unix domain socket, plus an Elixir integration test that proves the full transport chain: Gun HTTP/2 client → byte-pipe UDS relay → tonic server. The Health RPC round-trips with ok: true, confirming the assumption that grpc-elixir's Gun adapter can speak HTTP/2 over a relayed UDS without a VM. :gun added to deps (was an optional dep of :grpc, not yet resolved); grpc must be force-recompiled after adding gun so the conditional Gun adapter module is included. tonic-prost + tokio-stream added as dev-dependencies for the health_uds example. Integration tests excluded from the normal mix test run via :integration tag in test_helper.exs; run separately with --no-start + Application.ensure_all_started(:grpc) to avoid the suidhelper version check that fails on dev machines.
…op CBOR wire Replace the CBOR serve loop with a tonic 0.14 gRPC server bound on AF_VSOCK:1024. PID-1 invariants are preserved: sync mounts run before the tokio runtime, a tokio SIGCHLD reaper is spawned before the server accepts connections, and the main thread parks forever if serve() returns. - src/agent.rs: new GuestAgent impl; Health returns ok=true; Exec stubs UNIMPLEMENTED (Task 4 fills it in) - src/init.rs: drop raw libc::signal SIGCHLD handler; add spawn_reaper() using tokio::signal so tokio's child-process tracking is not clobbered - src/main.rs: PID-1 skeleton with tokio runtime + VsockListener::bind + serve_with_incoming(listener.incoming()) - src/exec.rs: move Request/Response types here (no serde/CBOR); drop serve_one(); run() core unchanged for Task 4 - tests/exec.rs: import from exec:: instead of wire:: - Delete src/wire.rs, tests/wire.rs and [[test]] wire entry - Cargo.toml: add tonic-prost to [dependencies]; drop ciborium/serde/ serde_bytes; add [profile.release] size profile (z/lto/1cu/abort/strip) — musl binary 7.08 MB → 1.09 MB
Replace the scaffolding Request/Response types in exec.rs with a pure run(argv, env, cwd) -> (i32, Vec<u8>, Vec<u8>) core; Agent::exec maps the generated ExecRequest -> run (via spawn_blocking) -> ExecResponse.
RelayDialer: pure module that dials the Firecracker vsock UDS via :socket, sends CONNECT <port>\n, reads the OK line byte-by-byte (to avoid consuming HTTP/2 bytes), and returns the connected socket. Relay: GenServer that listens on a host UDS, accepts connections into an unblocking linked acceptor process, and for each conn spawns an isolated connection process that calls RelayDialer.dial/3 then pipes bytes in two unlinked direction-workers — a pipe crash drops only that connection. terminate/2 closes the listen socket and removes the socket file.
…er-read test
Fix 1: distinguish :closed (expected terminate/2 shutdown) from any other
accept error in accept_loop. Non-:closed errors now exit({:accept_error, reason})
so the linked GenServer receives a non-:normal EXIT and stops, letting the
Task 7 supervisor restart the relay rather than leaving it zombied with no
acceptor. Add synthetic-EXIT test proving the GenServer stops on non-normal
acceptor exits.
Fix 2: add over-read regression test to relay_dialer_test — fake Firecracker
sends "OK 5\nHELLO_H2_BYTES" in one write; asserts trailing bytes are readable
after dial/3 returns, which would fail if a bulk recv replaced the byte-by-byte
line reader.
Fix 3: change pipe workers from spawn to spawn_link so a connection handler
death reaps both workers via the link instead of orphaning them. Keep the
existing Process.monitor for the handler to react to each worker's normal end.
Add Process.unlink(sibling) before Process.exit(sibling, :kill) in
await_pipe_end to prevent the :killed cascade from reaching the handler back
through the now-live link.
…lient
Replace Hyper.Node.FireVMM.Exec (CBOR over vsock) with a typed gRPC client
(Hyper.Node.FireVMM.Agent) that dials the per-VM relay UDS through the Gun
adapter. relay_socket_path/1 is the single source of truth for the host-side
socket path; the relay GenServer (Task 7) will call the same function.
Also fix Mix.Tasks.Compile.GrpcGen to format generated pb.ex files via
Code.format_string! instead of Mix.Task.run("format"), which is a no-op
when the format task was already consumed earlier in the same mix check run.
- Wire Relay as a third :one_for_one child of FireVMM.Supervisor, started
with {vsock_uds: Jailer.host_vsock/1, listen_path: Agent.relay_socket_path/1}.
- Override Relay.child_spec/1 with restart: :transient so abnormal acceptor
crashes are restarted by the supervisor while clean :shutdown is not.
- Add Reclaim.reclaim_sockets/0 (public @doc false) to sweep grpc-*.sock
files in socket_dir at node boot; called from Reclaim.run/0 alongside the
existing dm/loop sweeps. Also creates the dir if absent so the relay's
bind never races a missing directory on first boot.
…v/PATH in Hyper.exec
Replace the hand-written `map_grant_err` in `grant-api`/`grant-vsock` with
`impl From<GrantError>`, so the shared errors convert with `?`/`Error::from`.
Move the lexical `--socket` checks (absolute, strict components, exact leaf
name) out of `grant_{api,vsock}_under` and onto a validated `ApiSocket` /
`VsockSocket` newtype parsed by clap via `FromStr`, mirroring `jailer::JailSock`.
The parse stays pure (reads no config); the base-relative shape check (`<exec>/
<id>/root/<leaf>` depth) stays in the seam since it needs the runtime jail_base.
Add a manual `Clone` for `SafePath` (clap requires it on arg types; the phantom
axis markers stay non-Clone). Split the grant tests so the lexical refusal
contracts exercise the newtype's `FromStr` and the shape/behavior contracts
drive the seam with an already-validated socket.
Replace the hand-rolled tokio::signal SIGCHLD reaper with the pid1 crate. As PID 1, pid1-rs re-execs the agent as a child and runs signal forwarding and zombie/orphan reaping in the parent; the child mounts /proc, /sys, /dev and serves the vsock gRPC API as before. Prune the now-dead nix 'process' and tokio 'signal' features.
pid1's launch() re-execs via current_exe() (reads /proc/self/exe); running it before init::setup() mounted /proc panicked PID 1 into a reboot loop on real boots. Mount first, then hand off; the re-exec'd child inherits the mounts via the shared mount namespace.
The re-exec'd child re-enters main() and re-runs setup(); the earlier comment wrongly said setup runs exactly once. The extra run is a harmless best-effort re-mount over the inherited (shared-namespace) mounts.
The :guest_agent_build compiler runs `cargo build --target aarch64-unknown-linux-musl` with no linker override, so cargo fell back to the host cc and the aarch64 cross build failed to link (warn+skip). Point that target at the aarch64 cross-gcc. Only affects the aarch64 build; x86_64-only hosts still warn-and-skip harmlessly.
console=ttyS0 makes kernel printk synchronous at 115200 baud, blocking the boot vCPU on every line and dominating cold-boot wall time. Guest output goes over vsock, not the console, so a silent boot loses nothing operationally; debug boots re-add console=ttyS0 via the per-VM boot_args override.
Hyper.exec/3 becomes a facade: it resolves the VM's owning node (node(pid) for a pid, Routing.whereis/1 for a vm_id) and erpc-calls that node's new Hyper.Node.exec/3, which delegates to the FireVMM relay/agent. Accepts a pid or a vm_id binary; unresolvable -> :not_found, unreachable owning node -> :node_unreachable. Removes the v1 local-node assumption.
The prior two tests only hit the nil->:not_found short-circuit before route/4.
Register a stand-in {vm_id, :supervisor} so whereis/1 resolves locally, then
assert exec/3 routes through route/4 into Hyper.Node.exec (agent connect error)
rather than short-circuiting -- reason is neither :not_found nor
:node_unreachable. Polls Horde materialisation to avoid the CRDT-lag race.
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
1 participant
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.
No description provided.