Repository navigation
Restore green CI and fix VMM, lifecycle and tenant isolation bugs - #91
Open
abhishek-anand wants to merge 16 commits into
Open
abhishek-anand wants to merge 16 commits into
abhishek-anand wants to merge 16 commits into
Conversation
Rust 1.99 deprecates Atomic*::fetch_update, which fails the orch clippy gate under -D warnings. Use the infallible Atomic*::update (stable since 1.95, already the vmm MSRV) and declare that minimum for orch so the requirement is explicit. The PTY rejection counter now reuses the metrics helper instead of duplicating it. Bump rustls 0.23.45 -> 0.23.46 for RUSTSEC-2026-0346, -0347, -0348 and -0349, and replace the yanked chacha20 0.10.1 with 0.10.2.
The check job ran clippy, check and test without --locked while the release, KVM and musl builds use it, so a stale Cargo.lock passed PR CI and only failed at tag time. test/e2e.sh skipped unknown suite names without counting a failure, so a typo such as `test/e2e.sh smok` reported E2E_PASS with nothing run. Count unknown names as failures and require at least one pass.
The host dirty tracker holds packed snapshot offsets: KVM dirty logs, write_phys and live_snapshot all read it that way. Virtio devices, which only know guest physical addresses, marked raw GPAs into it. For guests with RAM above the MMIO aperture, a device write to high memory during pre-copy was either dropped (pfn beyond the packed size) or recorded against the wrong page, so fork children and diff restores could resume with stale RAM behind a completed used-ring entry. Add SoftwareDirtyBitmap::mark_guest_range, which splits the range at the aperture and packs high RAM after low RAM exactly as gpa_to_offset does, and use it at every device call site. Offset-based callers (UFFD, write_phys, balloon discard, dirty replay) keep mark_range. Also join the UFFD handler before closing its descriptor on drop. The handler only holds the raw fd number, so closing first allowed another thread to reuse it while the handler could still read or ioctl it.
The preflight decoded gzip layers with GzDecoder, which stops after the first member, while umoci extracts every member. A small first member passed all entry, size and expansion limits and hid an unbounded second member from the scan. Decode with MultiGzDecoder and drain everything after the tar end-of-archive marker through the expansion limit, rejecting anything other than zero padding. GNU sparse entries (old header form and PAX GNU.sparse.*) expand to a logical size the expansion budget never counted, and tar-rs does not decode the PAX form while umoci does. Reject both forms; Go-based image builders never emit them.
read_text_line checked its deadline only after a read timed out, so a peer sending one byte just inside each per-read timeout could hold the single accept thread for hours, up to the 64 KiB line cap, blocking the real agent connection and the clone-repair barrier. Check the deadline before every read and shrink the read timeout to the time remaining.
Any accept() error, such as EMFILE from a connection flood, ended the peer server task. The server-exit path treats that as a fatal server error, requests shutdown, and with the default TARIT_REAP_ON_SHUTDOWN stops every local VM. Log the error and keep accepting: per-connection failures retry at once, resource exhaustion backs off for one second as axum's serve loop does, and shutdown still interrupts the wait. Finished connection tasks were also only joined after the loop exited, so they accumulated for the life of the process. Reap them while the listener runs.
VmAdmissionGate::enter returned a guard of a single process-wide Mutex, and resume_vm holds it across the resume RPC, the readiness wait and rollback. One slow resume therefore serialized every other resume and create on the node and blocked tokio workers in /readyz, create admission and the autoscaler. The gate only has to guarantee that close() waits for admitted work and admits nothing afterwards. Make it an RwLock: enter() shares it, close() takes it exclusively after setting the closed flag, and enter() rejects before queueing behind a pending close. /readyz now reads the flag instead of entering the gate.
Live boots release their reservation on the next terminal transition, but a failed wake leaves the VM durably Hibernated, so nothing released it. Every later wake then hit AlreadyReserved and returned 409 until the VM was deleted or taritd restarted. Release the reservation when the restore task or publication fails, unless the supervisor still retains the boot for cleanup or the runtime is already running.
Only Creating, Running, Paused and Suspended VMs protected their base rootfs. A hibernated VM restores over its original base and every snapshot boots from the base recorded at capture time, so removing an image they referenced made wakes and restores fail permanently.
The only fixed destination guard in the per-VM forward rules dropped the VM pool (172.16.0.0/16). A tenant that opened 0.0.0.0/0, which is common for package installs, could also reach 169.254.169.254 and the rest of link-local through the uplink, including cloud instance metadata and its instance-role credentials. Program a fourth per-TAP guard, `ip daddr 169.254.0.0/16 drop`, after the lateral guard and before the uplink guard and every tenant accept, and require it wherever forward policy is validated. Existing hosts pick it up on the next taritd start: recovery already deletes every tagged rule for a slot and reprograms the full guard set before releasing quarantine. The egress API now rejects entries entirely inside link-local with 400 instead of persisting rules the host always drops. Stored policies are still loaded with the permissive normalizer so an old entry cannot block boot, resume or recovery.
SSH shell and exec channels attached VMM PTYs without taking the global, per-tenant or per-VM connection permits the WebSocket path enforces, and exec commands skipped validate_shell. Take the same permit before routing or attaching, hold it for the life of the channel, and validate the command first. The gateway also resolved the VM's owner node before checking ownership, so any key holder could learn a VM's peer host and RPC address, and raw internal, VMM and ownership error text reached the client. Authorize against the recorded owner first, make foreign VMs indistinguishable from missing ones, and send fixed public messages (reusing the API's public error mapping) while logging details on the server.
timeout_ms on execute was unchecked, and a synchronous exec waited on the VM's operation gate with no deadline. Past the API request deadline the client gets 504, but the handler keeps running and keeps its global in-flight permit, so one tenant sending enough long or queued execs to its own VM could exhaust every permit and turn all API calls, including admin deletes, into 429 until those commands finished. Reject timeout_ms above one hour on both execute endpoints, and above the API request deadline on synchronous execute (pointing callers to execute_async). Request-bound execs, including peer-forwarded ones, now wait for the operation gate only as long as the request deadline still leaves room for the command, then return 409 without recording an execution. Async execs keep queueing.
share_audit_fields loaded the share before authorization and copied its VM id, guest port and visibility into the PATCH, DELETE or token-issue audit event recorded under the caller's key. A tenant probing another tenant's share id could read those fields back from /v1/audit. Populate them only for admins and the owning tenant.
A replayed fork returns 200, but forkVm documented only 201. The TypeScript client accepted only 201 and the generated Python parser returned None for 200, so a retry with the same child id after a lost response reported failure while the child VM was running. Document the 200 replay, accept it in both clients, and retry Python requests on any httpx.TransportError so "server disconnected" is retried like other transport failures. AuditEvent.action lacked fork, hibernate, create_branch, update_branch, delete_branch and set_balloon, which the server records; the Python SDK raised on them. Add them and a test that keeps the enum equal to the recorded action constants. The spec also documents the new execute timeout limits. Clients are regenerated with sdk/generate.sh.
The server returns an opaque snapshot_id, but the CLI decoded a path and host_id, so `taritd vm snapshot` failed after the snapshot was taken.
tokio-postgres defaults to sslmode=prefer, which silently falls back to plaintext when the server or an on-path attacker declines TLS. Fleet state includes VM ownership used for cross-node authorization, so with TARIT_PRODUCTION=1 require sslmode=require unless every host is a local Unix socket. The check parses the URL exactly as the pool does and does not echo it in errors.
There was a problem hiding this comment.
🟡 Changes recommended
Execution deadline enforcement remains incomplete, and remote asynchronous jobs can fail instead of queuing behind busy VMs.
3 open issues
What changed in this PR
This PR aims to restore CI and strengthen VM correctness, lifecycle handling, tenant isolation, and client compatibility.
Changes:
- Updates dependencies and CI checks, with regression coverage.
- Fixes memory tracking, restore cleanup, lifecycle handling, and security controls.
- Aligns API contracts, SDKs, CLI output, and documentation.
| File | Description |
|---|---|
| vmm/crates/vmm-memory-backend/src/uffd_restore.rs | Joins the handler before closing its descriptor. |
| vmm/crates/vmm-memory-backend/src/dirty.rs | Maps guest dirty ranges to snapshot offsets. |
| vmm/crates/vmm-memory-backend/src/backend.rs | Tests high-memory dirty tracking. |
| vmm/crates/vmm-devices/src/virtio/vsock.rs | Corrects vsock dirty tracking. |
| vmm/crates/vmm-devices/src/virtio/vqueue.rs | Corrects queue dirty tracking and adds coverage. |
| vmm/crates/vmm-devices/src/virtio/rng_transport.rs | Corrects RNG dirty tracking. |
| vmm/crates/vmm-devices/src/virtio/net_transport.rs | Corrects network dirty tracking. |
| vmm/crates/vmm-devices/src/virtio/blk_transport.rs | Corrects block-device dirty tracking. |
| vmm/crates/vmm-core/src/vsock_exec.rs | Bounds banner reads with an absolute deadline. |
| vmm/crates/vmm-core/src/oci.rs | Hardens compressed-layer and sparse-entry validation. |
| test/e2e.sh | Rejects unknown suites and empty successful runs. |
| sdk/typescript/src/tarit.ts | Accepts successful fork replays. |
| sdk/typescript/src/generated/schema.ts | Regenerates API types and responses. |
| sdk/tests/typescript/high-level.test.ts | Tests fork replay after transport failure. |
| sdk/tests/python/test_high_level.py | Tests synchronous and asynchronous fork retries. |
| sdk/python/tarit_sdk/models/execute_request.py | Documents execution timeout limits. |
| sdk/python/tarit_sdk/models/audit_event_action.py | Adds recorded audit actions. |
| sdk/python/tarit_sdk/high_level.py | Retries transport errors. |
| sdk/python/tarit_sdk/api/default/fork_vm.py | Parses successful fork replays. |
| sdk/python/tarit_sdk/api/default/execute.py | Recognizes timeout-validation and conflict responses. |
| sdk/python/tarit_sdk/api/default/execute_async.py | Recognizes timeout-validation responses. |
| sdk/python-high-level/high_level.py | Updates the Python retry source template. |
| orch/README.md | Corrects snapshot CLI documentation. |
| orch/openapi.yaml | Documents execution limits, fork replay, and audit actions. |
| orch/docs/OPERATIONS.md | Documents mandatory link-local guards. |
| orch/docs/CONFIGURATION.md | Documents shared PTY limits and database TLS. |
| orch/docs/API.md | Documents execution and egress restrictions. |
| orch/crates/taritd/src/supervisor.rs | Allows concurrent admissions while preserving shutdown draining. |
| orch/crates/taritd/src/pty.rs | Shares admission and validation helpers with SSH. |
| orch/crates/taritd/src/ops.rs | Adds execution deadlines and failed-wake reservation cleanup. |
| orch/crates/taritd/src/openapi.rs | Checks audit-action schema consistency. |
| orch/crates/taritd/src/net.rs | Enforces link-local isolation and recovery validation. |
| orch/crates/taritd/src/metrics.rs | Updates saturating atomic counters. |
| orch/crates/taritd/src/main.rs | Retries peer accept errors and reaps connection tasks. |
| orch/crates/taritd/src/internal.rs | Adds peer execution gate deadlines. |
| orch/crates/taritd/src/image.rs | Protects hibernation and snapshot base images. |
| orch/crates/taritd/src/gateway.rs | Adds SSH ownership checks, PTY limits, and sanitized errors. |
| orch/crates/taritd/src/config.rs | Enforces production database transport requirements. |
| orch/crates/taritd/src/cli.rs | Decodes and prints snapshot handles. |
| orch/crates/taritd/src/api.rs | Bounds execution budgets and protects share audit metadata. |
| orch/crates/taritd/Cargo.toml | Inherits the workspace Rust requirement. |
| orch/crates/tarit-types/src/lib.rs | Defines the maximum execution timeout. |
| orch/crates/tarit-fleet/src/lib.rs | Validates database TLS requirements. |
| orch/Cargo.toml | Declares the minimum Rust version. |
| orch/Cargo.lock | Updates security-related dependencies. |
| .github/workflows/ci.yml | Uses locked dependencies for checks and tests. |
Review effort: Balanced
Comment on lines
+958
to
+959
| // The peer waits on this request, so it is bound by the same deadline. | ||
| let gate_deadline = ops::exec_gate_deadline(&state.config, body.timeout_ms); |
Comment on lines
+1862
to
+1863
| tokio::time::Instant::now() | ||
| + std::time::Duration::from_millis(config.api_request_timeout_ms.saturating_sub(timeout_ms)) |
| ensure_active_local(state, vm_id).await?; | ||
| let gate = state.supervisor.operation_gate(vm_id)?; | ||
| let _operation = gate.lock_owned().await; | ||
| let _operation = lock_operation_gate(gate, vm_id, gate_deadline).await?; |
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.

Summary
mainhas failed CI since 2026-10-05 and the Security workflow since the latest advisory sync. This change makes both green again and fixes the highest-impact correctness and isolation bugs found in a review ofvmm/,orch/, the SDKs and CI. Each fix is its own commit with a regression test that runs without KVM, root or PostgreSQL.CI and dependencies
Atomic*::fetch_update. Under-D warningsthis failed the orch clippy gate. The counters now useAtomic*::update, and orch declaresrust-version = "1.95", which matchesvmm/.--lockedon the CI check, clippy and test steps. Release, musl and KVM builds already use it.test/e2e.shfalse pass. It no longer reportsE2E_PASSfor an unknown suite name, or when nothing ran.VMM
mark_guest_range, which packs high RAM the same waygpa_to_offsetdoes.MultiGzDecoder. Previously the preflight stopped after the first gzip member while umoci extracts all of them, so a second member could bypass every size and entry limit.taritd lifecycle
accept()error, such as EMFILE under a connection flood, ended the peer server. That triggered node shutdown and, with the defaultTARIT_REAP_ON_SHUTDOWN, stopped every VM. Errors are now logged and retried with axum-style backoff, and finished connection tasks are reaped while the listener runs.VmAdmissionGatewas a single globalMutexthatresume_vmheld across the resume RPC and readiness wait. One slow resume serialized every create and resume and blocked tokio workers in/readyz. The gate is now anRwLockwith the same drain-on-close guarantee, and/readyzreads the flag.image rmandimage gcno longer delete base images that hibernated VMs or snapshots still restore from.Tenant isolation
169.254.0.0/16ahead of tenant allow rules. Before, any tenant that allowed0.0.0.0/0could reach cloud instance metadata and its credentials. The egress API rejects entries entirely inside link-local with 400. Policies stored before this change still load, so boot and resume are unaffected. Existing hosts get the guard on the next taritd start, because recovery already rebuilds every slot's guard set before releasing quarantine.validate_shell.API, SDK and fleet
timeout_mswas unchecked, and a synchronous exec waited on a busy VM's operation gate with no deadline. Past the request deadline the client gets 504, but the handler kept its global in-flight permit, so one tenant could exhaust every permit and make all API calls, admin deletes included, return 429. Now:timeout_msat one hour;httpx.TransportError.AuditEvent.actionenum gains the six actions the server already records. A test keeps it equal to the recorded constants.taritd vm snapshotdecodes thesnapshot_idthe server returns. Before, it failed after taking the snapshot.TARIT_PRODUCTION=1,TARIT_DATABASE_URLmust usesslmode=requireunless every host is a Unix socket. The tokio-postgres defaultprefersilently falls back to plaintext.Behaviour changes to note
169.254.0.0/16return 400. Guests can no longer reach link-local resolvers such as 169.254.169.253; taritd does not configure those by default.timeout_msabove one hour return 400, and so do synchronous ones aboveTARIT_API_REQUEST_TIMEOUT_MS(default 180000). Before, the sync case ended in 504 while the command kept running. A sync exec that cannot get a busy VM in time returns 409.TARIT_DATABASE_URLlackssslmode=requirerefuse to start.Not in this change
These are tracked separately because they need design work or hardware validation:
run_on_socketexits on any accept error);Checklist
vmm/,orch/,proto/):cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warningsandcargo test --workspacepass locally on Rust 1.99. The exception is two existingvmm-apisocket tests, which fail only on macOS because the checkout path exceedsSUN_LEN.cargo deny checkpasses for all three workspaces.unsafe. The existingSAFETYcomment onLazyRestoreis updated for the new drop order.sdk/generate.sh. A second run produces no diff. Python (verify_release, mypy --strict, ruff, unittest) and TypeScript (check, test, build) pass.