diff --git a/cmd/api/api/cp.go b/cmd/api/api/cp.go index bfa64e45c..67d054efc 100644 --- a/cmd/api/api/cp.go +++ b/cmd/api/api/cp.go @@ -104,6 +104,11 @@ func (s *ApiService) CpHandler(w http.ResponseWriter, r *http.Request) { return } + if inst.MacOS != nil && !inst.GuestAgentEnabled() { + http.Error(w, `{"code":"unsupported","message":"file copy requires the shared macOS guest agent to be enabled"}`, http.StatusNotImplemented) + return + } + if inst.State != instances.StateRunning { http.Error(w, fmt.Sprintf(`{"code":"invalid_state","message":"instance must be running (current state: %s)"}`, inst.State), http.StatusConflict) return diff --git a/cmd/api/api/exec.go b/cmd/api/api/exec.go index 9313e5372..60a3827fb 100644 --- a/cmd/api/api/exec.go +++ b/cmd/api/api/exec.go @@ -71,8 +71,8 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) { return } - if inst.MacOS != nil { - http.Error(w, `{"code":"unsupported","message":"exec is not implemented for experimental macOS instances"}`, http.StatusNotImplemented) + if inst.MacOS != nil && !inst.GuestAgentEnabled() { + http.Error(w, `{"code":"unsupported","message":"exec is not implemented for macOS images without the shared guest agent enabled"}`, http.StatusNotImplemented) return } @@ -128,6 +128,8 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) { tracer := otel.Tracer("hypeman/exec") ctx, span := tracer.Start(ctx, "exec.session", trace.WithAttributes(execSpanAttributes(inst.Id, execReq.TTY)...)) defer span.End() + ctx, cancel := context.WithCancel(ctx) + defer cancel() // Audit log: exec session started log.InfoContext(ctx, "exec session started", @@ -146,11 +148,10 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) { var resizeChan chan *guest.WindowSize if execReq.TTY { resizeChan = make(chan *guest.WindowSize, 10) - defer close(resizeChan) } // Create WebSocket read/writer wrapper that handles resize messages - wsConn := &wsReadWriter{ws: ws, ctx: ctx, resizeChan: resizeChan} + wsConn := &wsReadWriter{ws: ws, ctx: ctx, resizeChan: resizeChan, cancel: cancel} dialer, err := s.InstanceManager.GetVsockDialer(ctx, inst.Id) if err != nil { @@ -221,6 +222,7 @@ type wsReadWriter struct { reader io.Reader mu sync.Mutex resizeChan chan<- *guest.WindowSize // Channel to send resize events (nil if not TTY) + cancel context.CancelFunc // ends the exec session when the websocket side fails } func (w *wsReadWriter) Read(p []byte) (n int, err error) { @@ -241,6 +243,7 @@ func (w *wsReadWriter) Read(p []byte) (n int, err error) { // Read next WebSocket message messageType, data, err := w.ws.ReadMessage() if err != nil { + w.cancel() return 0, err } @@ -273,6 +276,7 @@ func (w *wsReadWriter) Read(p []byte) (n int, err error) { func (w *wsReadWriter) Write(p []byte) (n int, err error) { if err := w.ws.WriteMessage(websocket.BinaryMessage, p); err != nil { + w.cancel() return 0, err } return len(p), nil diff --git a/cmd/api/api/exec_disconnect_test.go b/cmd/api/api/exec_disconnect_test.go new file mode 100644 index 000000000..ad16654f5 --- /dev/null +++ b/cmd/api/api/exec_disconnect_test.go @@ -0,0 +1,45 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/require" +) + +func TestExecWebsocketDisconnectCancelsSession(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + readDone := make(chan error, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + readDone <- err + return + } + defer conn.Close() + wrapped := &wsReadWriter{ws: conn, ctx: ctx, cancel: cancel} + _, err = wrapped.Read(make([]byte, 1)) + readDone <- err + })) + defer server.Close() + conn, _, err := websocket.DefaultDialer.Dial("ws"+strings.TrimPrefix(server.URL, "http"), nil) + require.NoError(t, err) + require.NoError(t, conn.Close()) + select { + case <-ctx.Done(): + case <-time.After(5 * time.Second): + t.Fatal("WebSocket disconnect did not cancel exec session") + } + select { + case err := <-readDone: + require.Error(t, err) + case <-time.After(5 * time.Second): + t.Fatal("WebSocket reader did not exit") + } +} diff --git a/cmd/api/api/macos_test.go b/cmd/api/api/macos_test.go index a7de011ac..cc8ced902 100644 --- a/cmd/api/api/macos_test.go +++ b/cmd/api/api/macos_test.go @@ -46,6 +46,36 @@ func TestMacOSStatRejectedBeforeGuestDial(t *testing.T) { require.Equal(t, "unsupported", unsupported.Code) } +func TestMacOSGuestAgentAdmissionBeforeUpgrade(t *testing.T) { + for _, tc := range []struct { + name string + declared, skipped bool + status int + }{ + {"unmanaged", false, false, http.StatusNotImplemented}, + {"disabled", true, true, http.StatusNotImplemented}, + {"enabled but stopped", true, false, http.StatusConflict}, + } { + for _, operation := range []string{"exec", "cp"} { + t.Run(tc.name+"/"+operation, func(t *testing.T) { + s := &ApiService{} + inst := &instances.Instance{StoredMetadata: instances.StoredMetadata{ + MacOS: &images.MacOSImage{GuestAgent: tc.declared}, SkipGuestAgent: tc.skipped, + }, State: instances.StateStopped} + ctx := mw.WithResolvedInstance(context.Background(), "test", inst) + r := httptest.NewRequest(http.MethodGet, "/instances/test/"+operation, nil).WithContext(ctx) + w := httptest.NewRecorder() + if operation == "exec" { + s.ExecHandler(w, r) + } else { + s.CpHandler(w, r) + } + require.Equal(t, tc.status, w.Code) + }) + } + } +} + func TestMacOSSchemaDefersTemplateDefaults(t *testing.T) { spec, err := oapi.GetSwagger() require.NoError(t, err) diff --git a/docs/macos-experimental.md b/docs/macos-experimental.md index 187542f00..f1378645b 100644 --- a/docs/macos-experimental.md +++ b/docs/macos-experimental.md @@ -72,14 +72,19 @@ attached. NAT uses the preserved MAC. The IP is observed from the host's VZ DHCP leases, which may retain an old lease before the guest is ready. **`Running` means the VMM is running, not that SSH, the desktop or Chrome is -ready.** Readiness is currently tested over SSH. Startup does not launch Chrome. +ready.** Unmanaged templates still need an external readiness check. Templates +provisioned with the shared Darwin GuestService can opt in with `guest_agent: true` +in their platform configuration; system readiness is observed separately through +`guest_agent_ready_at`. See [Darwin GuestService](macos-guest-agent.md). Startup +does not launch Chrome. Guest identity, host keys, user secrets and MAC are preserved. Only one active instance with a given Mac identifier is allowed. Do not run the source bundle or an external clone concurrently. Production provisioning needs identity and credential rekeying; this spike is not a multi-tenant image format. -`POST /instances/{id}/stop` shuts down the VMM; without a Darwin guest agent, -it does **not** guarantee an orderly guest OS/application shutdown. Use a human +`POST /instances/{id}/stop` attempts orderly GuestService shutdown for opted-in +images, then falls back to VMM shutdown if necessary. Without an enabled Darwin +guest agent it does **not** guarantee an orderly guest OS/application shutdown. Use a human or an authorized guest shutdown workflow before destructive operations when application consistency matters. Start cold-boots the instance's existing disk. Delete removes instance storage, not the imported image. Deleting the imported @@ -87,8 +92,9 @@ image does not affect existing instances: each owns independent copies and starts without the template. Unsupported instance operations reject requests: snapshot/fork/standby/restore, -updates, volumes, env/commands/credential brokering, Linux guest-agent exec and -vsock operations, health/restart/auto-standby policies, passthrough and shaping. +updates, volumes, startup env/commands/credential brokering, +health/restart/auto-standby policies, passthrough and shaping. Exec/file-copy over +vsock require an opted-in shared agent; unmanaged images reject those requests. The shim's standalone save/restore proof does not make API snapshot semantics safe; consistent disk+aux+state bundle handling remains separate work. diff --git a/docs/macos-guest-agent.md b/docs/macos-guest-agent.md new file mode 100644 index 000000000..61f8a3a68 --- /dev/null +++ b/docs/macos-guest-agent.md @@ -0,0 +1,117 @@ +# Experimental Darwin GuestService + +The existing guest-agent executable now builds for macOS and serves the same +`guest.GuestService` gRPC contract as Linux on vsock port **2222**. It reuses +exec, copy-to/from-guest, and stat implementations; this is not the browser +prototype's custom HTTP protocol or its ports. + +## Build + +Build on a macOS development machine with the macOS SDK and cgo enabled: + +```sh +CGO_ENABLED=1 GOOS=darwin GOARCH=arm64 go build \ + -o guest-agent-darwin-arm64 ./lib/system/guest_agent +``` + +Darwin native AF_VSOCK requires cgo. A cgo-disabled Darwin executable builds but +refuses to listen with an explicit error. Linux keeps its existing Go vsock +listener and cgo-disabled build path. The Darwin listener accepts only host +CID 2, marks descriptors close-on-exec, and provides net.Conn deadlines for +gRPC. Do not start the executable on the development host to test guest services. + +## Provisioning boundary + +Install the binary **inside a stopped-template provisioning guest**, not on the +host. System operations need a root LaunchDaemon. Use a root-owned, non-writable +binary location and launchd configuration; provision permissions and logs +explicitly. The default readiness file is `/var/run/hypeman/guest-agent-ready` +(`HYPEMAN_AGENT_READY_FILE` can override it). + +A listening system agent does not establish autologin, desktop readiness, TCC +permissions, or browser readiness. Root and user desktop agents need a reviewed +handoff and explicit session selection before desktop execution is supported. +No desktop agent installer or public session-selector extension is part of this +slice. + +The host API and hypervisor still enforce authorization. The vsock host-CID +check is transport admission, not a replacement for instance authority checks. +As with the Linux guest agent, commands and file operations run as root with no +per-caller credential switch; the host API alone decides who may reach them. +An exec `timeout_seconds` ends the command; it does not bound a healthy stream. +Do not expose this privileged service through unauthenticated host forwarding. + +## Image declaration and host integration + +Set `"guest_agent": true` in the image's experimental macOS platform configuration +only after provisioning the root shared agent on vsock 2222. This field is omitted +by default, so existing local templates remain unmanaged. The normal instance +`skip_guest_agent` option can disable agent integration even for a declared image. + +For declared/enabled instances, the normal exec and file-copy handlers use the +shared GuestService and stop attempts its Shutdown RPC before the existing +forced-stop fallback. An unavailable agent still produces a timeout/failure, +not an assertion that it is ready. Readiness probing records the existing +`guest_agent_ready_at` marker without requiring or inventing a Linux workload +start marker. `Running` remains VMM-running for macOS; it does not certify system, +desktop or browser readiness. This readiness timestamp is a boot observation, +not continuous agent health. Start clears the prior boot's readiness marker. + +Undeclared/disabled images reject exec/copy before WebSocket upgrade and skip +agent shutdown/probes. The declaration is an image capability claim, not a +verified live guest handshake or a security credential. Runtime connectivity +and privileged provisioning still require the validation below. + +## OS-specific operations + +- **Shutdown:** root-only `/sbin/shutdown -h now`, not a signal to launchd/PID 1. + Signal 0 (default) and SIGTERM mean orderly shutdown; other signals are rejected. + Permission, cancellation, and command errors are reported. An RPC response is + not proof the VMM has exited: the host must wait for teardown. +- **Network identity reconfiguration:** returns gRPC `Unimplemented` on Darwin. + Current VZ NAT uses guest DHCP; no static-address, MAC-rekey, ingress, or network + policy parity is claimed. +- **GPU status:** Linux NVIDIA initialization reporting is not macOS graphics + readiness. A Darwin guest without that device reports the existing unknown + state. + +## Validation and remaining integration + +In-process gRPC tests exercise exec stdout/stderr/exit/env, disconnect +cancellation, and file copy/stat round-trips. Darwin-specific tests exercise +shutdown policy without executing a real shutdown, explicit network rejection, +and transport deadline errors using a local socket pair. These do not prove a +live guest AF_VSOCK handshake for this executable. + +## Isolated live validation (2026-10-10) + +A manually provisioned root LaunchDaemon in the isolated QA guest survives normal +API cold boots. Authenticated normal API exec reports UID0 in both TTY modes; +private root-owned copy-to/from round trips and command/descendant timeout cleanup +pass repeated race-instrumented client tests. The readiness probe uses +`/usr/bin/true` on Darwin (`/bin/true` remains the Linux command); a live regression +first failed with the Linux path, then passed after this correction. A normal GET +persisted `GuestAgentReadyAt` while leaving `ProgramStartedAt` absent. + +Darwin can disconnect vsock while shutdown is in progress, before the RPC reply +arrives. The host therefore retains the configured grace period on Darwin even +when that reply is unavailable, instead of Linux's short failure fallback. Success +still requires the owned VMM process to exit. A live normal API stop confirmed +shutdown RPC acceptance, VMM exit and closed storage without forced fallback after +the correction. HTTP stop200 or `Stopped` alone is not a build/export receipt. + +This is manual provisioning of one QA instance, not automatic image installation, +public macOS builds, output publication or reusable-image sanitation proof. The +immutable imported base remains unchanged and does not acquire this capability. + +Remaining gates: + +- Reusable image provisioning and guest-agent version compatibility. +- Strict build/export stop receipts and API recovery with the root deployment; + bounded readiness wait semantics. +- Root/desktop session authorization and image provisioning. +- Broader backpressure/large-output validation of bounded non-TTY streaming, + PTY/disconnect and descendant-process cleanup, transfer failure/size handling, + and privilege/logging security review. +- Linux test execution on an appropriate runner, and independent authenticated + review. diff --git a/lib/guest/client.go b/lib/guest/client.go index 61376bf02..38b2b26a5 100644 --- a/lib/guest/client.go +++ b/lib/guest/client.go @@ -442,6 +442,8 @@ func retryableConnectionErrorType(err error) string { // execIntoInstanceOnce executes command in instance via vsock using gRPC (single attempt). func execIntoInstanceOnce(ctx context.Context, dialer hypervisor.VsockDialer, opts ExecOptions) (*ExitStatus, error) { + ctx, cancel := context.WithCancel(ctx) + defer cancel() start := time.Now() var bytesSent int64 @@ -515,14 +517,21 @@ func execIntoInstanceOnce(ctx context.Context, dialer hypervisor.VsockDialer, op // Handle resize events in background (if channel provided) if opts.ResizeChan != nil { go func() { - for resize := range opts.ResizeChan { - streamMu.Lock() - stream.Send(&ExecRequest{ - Request: &ExecRequest_Resize{ - Resize: resize, - }, - }) - streamMu.Unlock() + for { + select { + case <-ctx.Done(): + return + case resize, ok := <-opts.ResizeChan: + if !ok { + return + } + streamMu.Lock() + err := stream.Send(&ExecRequest{Request: &ExecRequest_Resize{Resize: resize}}) + streamMu.Unlock() + if err != nil { + return + } + } } }() } diff --git a/lib/images/types.go b/lib/images/types.go index b2c8ac272..f49437ef9 100644 --- a/lib/images/types.go +++ b/lib/images/types.go @@ -36,6 +36,9 @@ type MacOSImage struct { MAC string `json:"mac"` CPUs uint `json:"cpus"` Memory uint64 `json:"memory"` + // GuestAgent declares a provisioned system GuestService on vsock 2222. + // Readiness is probed separately; old templates remain unmanaged. + GuestAgent bool `json:"guest_agent,omitempty"` } // Validate checks the platform fields every macOS bundle must carry, whether it diff --git a/lib/instances/guest_agent_ready_command_test.go b/lib/instances/guest_agent_ready_command_test.go new file mode 100644 index 000000000..ef9ad91e1 --- /dev/null +++ b/lib/instances/guest_agent_ready_command_test.go @@ -0,0 +1,13 @@ +package instances + +import ( + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/stretchr/testify/require" +) + +func TestGuestAgentReadyExecutableMatchesGuestOS(t *testing.T) { + require.Equal(t, "/bin/true", guestAgentReadyExecutable(&StoredMetadata{})) + require.Equal(t, "/usr/bin/true", guestAgentReadyExecutable(&StoredMetadata{MacOS: &images.MacOSImage{}})) +} diff --git a/lib/instances/macos.go b/lib/instances/macos.go index c02eb32d0..9e42e61d7 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -29,7 +29,7 @@ func prepareMacOSCreate(req *CreateInstanceRequest, img *images.Image, caps hype return fmt.Errorf("%w: incomplete macOS image", ErrImageNotReady) } if req.HotplugSize != 0 || req.OverlaySize != 0 || len(req.Volumes) != 0 || len(req.Devices) != 0 || req.GPU != nil || len(req.Env) != 0 || len(req.Entrypoint) != 0 || len(req.Cmd) != 0 || req.NetworkEgress != nil || len(req.Credentials) != 0 || req.AutoStandby != nil || req.HealthCheck != nil || req.RestartPolicy != nil || req.SnapshotPolicy != nil || req.DiskIOBps != 0 || req.NetworkBandwidthDownload != 0 || req.NetworkBandwidthUpload != 0 { - return fmt.Errorf("%w: experimental macOS supports local disk clone, CPU/RAM, tags, expiration and NAT only; Linux commands/env/volumes, overlays, agents, policies and I/O shaping are unsupported", ErrInvalidRequest) + return fmt.Errorf("%w: experimental macOS supports local disk clone, CPU/RAM, tags, expiration and NAT only; Linux startup commands/env/volumes, overlays, policies and I/O shaping are unsupported", ErrInvalidRequest) } size, vcpus := req.Size, req.Vcpus if size == 0 { @@ -43,7 +43,7 @@ func prepareMacOSCreate(req *CreateInstanceRequest, img *images.Image, caps hype } req.Size, req.Vcpus = size, vcpus req.OverlaySize = *img.SizeBytes // Reserve the writable boot disk, not a Linux overlay. - req.SkipGuestAgent = true + req.SkipGuestAgent = req.SkipGuestAgent || !img.MacOS.GuestAgent req.SkipKernelHeaders = true return nil } @@ -100,6 +100,13 @@ func (m *manager) checkMacOSIdentityAvailable(ctx context.Context, stored *Store return nil } +// GuestAgentEnabled reports whether exec, copy, readiness and shutdown may use the +// shared guest agent. A macOS image must declare it; hand-written metadata that +// leaves SkipGuestAgent unset does not enable it. +func (s *StoredMetadata) GuestAgentEnabled() bool { + return !s.SkipGuestAgent && (s.MacOS == nil || s.MacOS.GuestAgent) +} + // rejectMacOS refuses operations the experimental macOS guest does not implement. // Callers check it right after loading the record they already hold under the lock. func (s *StoredMetadata) rejectMacOS(operation string) error { diff --git a/lib/instances/macos_test.go b/lib/instances/macos_test.go index ba44e5b0b..566958b15 100644 --- a/lib/instances/macos_test.go +++ b/lib/instances/macos_test.go @@ -35,6 +35,94 @@ func TestMacOSRequestDefaultsAndRejections(t *testing.T) { } require.ErrorIs(t, prepareMacOSCreate(&CreateInstanceRequest{}, testMacImage(), hypervisor.Capabilities{}), ErrInvalidRequest) } +func TestMacOSGuestAgentOptIn(t *testing.T) { + caps := hypervisor.Capabilities{SupportsMacOSBoot: true} + image := testMacImage() + image.MacOS.GuestAgent = true + request := CreateInstanceRequest{} + require.NoError(t, prepareMacOSCreate(&request, image, caps)) + require.False(t, request.SkipGuestAgent) + request = CreateInstanceRequest{SkipGuestAgent: true} + require.NoError(t, prepareMacOSCreate(&request, image, caps)) + require.True(t, request.SkipGuestAgent) +} + +func TestMacOSAgentReadinessSeparateFromRunning(t *testing.T) { + for _, tc := range []struct { + name string + declared, skipped, ready bool + }{ + {"legacy image", false, false, false}, + {"disabled", true, true, false}, + {"not ready", true, false, false}, + {"ready", true, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + image := testMacImage() + image.MacOS.GuestAgent = tc.declared + stored := StoredMetadata{Id: "mac", MacOS: image.MacOS, SkipGuestAgent: tc.skipped} + now := time.Now().UTC() + calls := 0 + m := &manager{now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { + calls++ + return tc.ready + }} + require.Equal(t, StateRunning, deriveRunningState(&stored)) + hydrated := m.hydrateBootMarkersFromLogs(context.Background(), &stored) + require.Equal(t, tc.ready, hydrated) + require.Equal(t, StateRunning, deriveRunningState(&stored)) + require.Nil(t, stored.ProgramStartedAt, "macOS does not invent a Linux workload marker") + if tc.ready { + require.Equal(t, &now, stored.GuestAgentReadyAt) + } else { + require.Nil(t, stored.GuestAgentReadyAt) + } + if tc.declared && !tc.skipped { + require.Equal(t, 1, calls) + } else { + require.Zero(t, calls) + } + }) + } +} + +func TestMacOSAgentReadyMarkerPersisted(t *testing.T) { + p := paths.New(t.TempDir()) + now := time.Now().UTC() + m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }} + require.NoError(t, m.ensureDirectories("mac")) + image := testMacImage() + image.MacOS.GuestAgent = true + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{Id: "mac", DataDir: p.InstanceDir("mac"), MacOS: image.MacOS}})) + m.persistBootMarkers(context.Background(), "mac") + meta, err := m.loadMetadata("mac") + require.NoError(t, err) + require.Equal(t, &now, meta.GuestAgentReadyAt) + require.Nil(t, meta.ProgramStartedAt) + require.True(t, meta.MacOS.GuestAgent) +} + +func TestMacOSAgentReadyMarkerPersistedByPublicRead(t *testing.T) { + p := paths.New(t.TempDir()) + now := time.Now().UTC() + m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }} + require.NoError(t, m.ensureDirectories("mac")) + socket := filepath.Join(p.InstanceDir("mac"), "vz.sock") + require.NoError(t, os.WriteFile(socket, nil, 0600)) + image := testMacImage() + image.MacOS.GuestAgent = true + stored := StoredMetadata{Id: "mac", DataDir: p.InstanceDir("mac"), SocketPath: socket, MacOS: image.MacOS, HypervisorType: hypervisor.TypeVZ, CreatedAt: now} + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + m.storeCachedHypervisorState("mac", hypervisor.StateRunning) + + inst, err := m.GetInstance(context.Background(), "mac") + require.NoError(t, err) + require.NotNil(t, inst.GuestAgentReadyAt) + meta, err := m.loadMetadata("mac") + require.NoError(t, err) + require.NotNil(t, meta.GuestAgentReadyAt, "readiness must reach metadata through the normal read path") +} + func TestMacOSBootConfigNoLinuxDevices(t *testing.T) { p := paths.New(t.TempDir()) m := &manager{paths: p} diff --git a/lib/instances/metrics.go b/lib/instances/metrics.go index 8d86b6cc7..d1ad9a9fa 100644 --- a/lib/instances/metrics.go +++ b/lib/instances/metrics.go @@ -603,7 +603,7 @@ func timeToRunningReadyAt(stored *StoredMetadata) *time.Time { if stored == nil || stored.ProgramStartedAt == nil { return nil } - if stored.SkipGuestAgent || stored.GuestAgentReadyAt == nil { + if !stored.GuestAgentEnabled() || stored.GuestAgentReadyAt == nil { return stored.ProgramStartedAt } if stored.GuestAgentReadyAt.After(*stored.ProgramStartedAt) { diff --git a/lib/instances/query.go b/lib/instances/query.go index 02ec7ed49..27a0afeb1 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -137,7 +137,7 @@ func (m *manager) deriveStateWithOptions(ctx context.Context, stored *StoredMeta return stateResult{State: StateCreated} case hypervisor.StateRunning: hydrated := false - if hydrateBootMarkers && stored.MacOS == nil { + if hydrateBootMarkers { hydrated = m.hydrateBootMarkersFromLogs(ctx, stored) } return stateResult{ @@ -219,7 +219,7 @@ func (m *manager) updateCachedHypervisorStateFromInstance(inst *Instance) { } func deriveRunningState(stored *StoredMetadata) State { - // For unmanaged macOS desktops, Running means VMM running, not app/agent ready. + // For macOS, Running means VMM running. Agent readiness is a separate marker. if stored.MacOS != nil { return StateRunning } @@ -293,8 +293,7 @@ func advancePhaseIfRunning(stored *StoredMetadata) { // services do not need to forward stdout/stderr to the serial console. // Returns true when at least one missing marker was found and populated. func (m *manager) hydrateBootMarkersFromLogs(ctx context.Context, stored *StoredMetadata) bool { - needProgram := stored.ProgramStartedAt == nil - needAgent := !stored.SkipGuestAgent && stored.GuestAgentReadyAt == nil + needProgram, needAgent := bootMarkersMissing(stored) if !needProgram && !needAgent { m.clearBootMarkerRescan(stored.Id) return false @@ -308,7 +307,42 @@ func (m *manager) hydrateBootMarkersFromLogs(ctx context.Context, stored *Stored ) defer span.End() - programStartedAt, guestAgentReadyAt := m.parseBootMarkers(ctx, stored.Id, needProgram, needAgent, stored.StartedAt) + hydrated := m.applyBootMarkers(ctx, stored) + if hydrated { + advancePhaseIfRunning(stored) + m.clearBootMarkerRescan(stored.Id) + } else { + m.deferBootMarkerRescan(stored.Id) + } + return hydrated +} + +// programMarkerSettled reports that no workload-start marker is awaited or already +// recorded. macOS guests have no Linux workload marker, so it is never awaited. +func (s *StoredMetadata) programMarkerSettled() bool { + return s.MacOS != nil || s.ProgramStartedAt != nil +} + +// bootMarkersMissing reports which boot markers stored still lacks. +func bootMarkersMissing(stored *StoredMetadata) (needProgram, needAgent bool) { + needProgram = !stored.programMarkerSettled() + needAgent = stored.GuestAgentEnabled() && stored.GuestAgentReadyAt == nil + return needProgram, needAgent +} + +// applyBootMarkers fills missing boot markers on stored from serial logs, then from +// the guest-agent probe, and reports whether it filled any. Callers own locking, +// phase advancement and persistence. +func (m *manager) applyBootMarkers(ctx context.Context, stored *StoredMetadata) bool { + needProgram, needAgent := bootMarkersMissing(stored) + if !needProgram && !needAgent { + return false + } + + var programStartedAt, guestAgentReadyAt *time.Time + if stored.MacOS == nil { + programStartedAt, guestAgentReadyAt = m.parseBootMarkers(ctx, stored.Id, needProgram, needAgent, stored.StartedAt) + } hydrated := false if needProgram && programStartedAt != nil { stored.ProgramStartedAt = programStartedAt @@ -318,15 +352,9 @@ func (m *manager) hydrateBootMarkersFromLogs(ctx context.Context, stored *Stored stored.GuestAgentReadyAt = guestAgentReadyAt hydrated = true } - if needAgent && stored.GuestAgentReadyAt == nil && stored.ProgramStartedAt != nil && m.hydrateGuestAgentReadyFromProbe(ctx, stored) { + if needAgent && stored.GuestAgentReadyAt == nil && stored.programMarkerSettled() && m.hydrateGuestAgentReadyFromProbe(ctx, stored) { hydrated = true } - if hydrated { - advancePhaseIfRunning(stored) - m.clearBootMarkerRescan(stored.Id) - } else { - m.deferBootMarkerRescan(stored.Id) - } return hydrated } @@ -421,7 +449,7 @@ func (m *manager) nowUTC() time.Time { } func (m *manager) hydrateGuestAgentReadyFromProbe(ctx context.Context, stored *StoredMetadata) bool { - if stored == nil || stored.SkipGuestAgent || stored.GuestAgentReadyAt != nil { + if stored == nil || !stored.GuestAgentEnabled() || stored.GuestAgentReadyAt != nil { return false } probe := m.guestAgentReadyProbe @@ -437,7 +465,7 @@ func (m *manager) hydrateGuestAgentReadyFromProbe(ctx context.Context, stored *S } func probeGuestAgentReady(ctx context.Context, stored *StoredMetadata) bool { - if stored == nil || stored.SkipGuestAgent { + if stored == nil || !stored.GuestAgentEnabled() { return false } dialer, err := hypervisor.NewVsockDialer(stored.HypervisorType, stored.VsockSocket, stored.VsockCID) @@ -449,13 +477,21 @@ func probeGuestAgentReady(ctx context.Context, stored *StoredMetadata) bool { defer cancel() exit, err := guest.ExecIntoInstance(probeCtx, dialer, guest.ExecOptions{ - Command: []string{"/bin/true"}, + Command: []string{guestAgentReadyExecutable(stored)}, Timeout: int32(guestAgentReadyProbeTimeout / time.Second), WaitForAgent: guestAgentReadyProbeWait, }) return err == nil && exit != nil && exit.Code == 0 } +func guestAgentReadyExecutable(stored *StoredMetadata) string { + // Darwin provides true in /usr/bin, not /bin. Keep the Linux probe unchanged. + if stored.MacOS != nil { + return "/usr/bin/true" + } + return "/bin/true" +} + // appLogPathsForMarkerScan returns app log paths in chronological order // (oldest rotated file to newest active file). func (m *manager) appLogPathsForMarkerScan(id string) []string { @@ -642,27 +678,7 @@ func (m *manager) persistBootMarkers(ctx context.Context, id string) { if err != nil { return } - - needProgram := meta.ProgramStartedAt == nil - needAgent := !meta.SkipGuestAgent && meta.GuestAgentReadyAt == nil - if !needProgram && !needAgent { - return - } - - programStartedAt, guestAgentReadyAt := m.parseBootMarkers(ctx, id, needProgram, needAgent, meta.StartedAt) - updated := false - if needProgram && programStartedAt != nil { - meta.ProgramStartedAt = programStartedAt - updated = true - } - if needAgent && guestAgentReadyAt != nil { - meta.GuestAgentReadyAt = guestAgentReadyAt - updated = true - } - if needAgent && meta.GuestAgentReadyAt == nil && meta.ProgramStartedAt != nil && m.hydrateGuestAgentReadyFromProbe(ctx, &meta.StoredMetadata) { - updated = true - } - if !updated { + if !m.applyBootMarkers(ctx, &meta.StoredMetadata) { return } diff --git a/lib/instances/shutdown_wait_test.go b/lib/instances/shutdown_wait_test.go new file mode 100644 index 000000000..1f9449680 --- /dev/null +++ b/lib/instances/shutdown_wait_test.go @@ -0,0 +1,18 @@ +package instances + +import ( + "testing" + "time" + + "github.com/kernel/hypeman/lib/images" + "github.com/stretchr/testify/require" +) + +func TestGuestShutdownWaitPreservesDarwinGraceOnLostReply(t *testing.T) { + linux := &Instance{} + mac := &Instance{StoredMetadata: StoredMetadata{MacOS: &images.MacOSImage{}}} + require.Equal(t, 500*time.Millisecond, guestShutdownWaitTimeout(linux, 30, false)) + require.Equal(t, 30*time.Second, guestShutdownWaitTimeout(linux, 30, true)) + require.Equal(t, 30*time.Second, guestShutdownWaitTimeout(mac, 30, false)) + require.Equal(t, 30*time.Second, guestShutdownWaitTimeout(mac, 30, true)) +} diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 603b9a75e..3a2973fdf 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -39,12 +39,12 @@ func hasVFIODevices(stored *StoredMetadata) bool { return storedVGPUDevicePath(stored) != "" || len(stored.Devices) > 0 } -// tryGracefulGuestShutdown asks guest init to shut down and waits for the +// tryGracefulGuestShutdown asks GuestService to shut down and waits for the // hypervisor process to exit. Returns true if the process exited in time. func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, stopTimeout int) bool { log := logger.FromContext(ctx) - if inst.SkipGuestAgent { + if !inst.StoredMetadata.GuestAgentEnabled() { log.DebugContext(ctx, "guest-agent disabled, skipping graceful guest shutdown", "instance_id", inst.Id) return false } @@ -84,7 +84,7 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, // Drop potentially stale pooled connection and retry once. guest.CloseConn(dialer.Key()) if retryErr := sendShutdown(); retryErr != nil { - log.WarnContext(ctx, "shutdown RPC failed; falling back to hypervisor shutdown", "instance_id", inst.Id, "error", retryErr) + log.WarnContext(ctx, "shutdown RPC response unavailable; awaiting guest power-off", "instance_id", inst.Id, "error", retryErr) } else { shutdownSent = true } @@ -92,11 +92,7 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, shutdownSent = true } - waitTimeout := time.Duration(stopTimeout) * time.Second - if !shutdownSent && waitTimeout > shutdownFailureFallbackWait { - // If we couldn't signal the guest, don't burn the full graceful timeout. - waitTimeout = shutdownFailureFallbackWait - } + waitTimeout := guestShutdownWaitTimeout(inst, stopTimeout, shutdownSent) if WaitForProcessExit(pid, waitTimeout) { log.DebugContext(ctx, "VM shut down gracefully", "instance_id", inst.Id) @@ -107,6 +103,16 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, return false } +func guestShutdownWaitTimeout(inst *Instance, stopTimeout int, shutdownSent bool) time.Duration { + wait := time.Duration(stopTimeout) * time.Second + // Darwin may close vsock before the reply while power-off is in progress. + // Preserve its grace period; success still requires actual owned VMM exit. + if !shutdownSent && inst.MacOS == nil && wait > shutdownFailureFallbackWait { + return shutdownFailureFallbackWait + } + return wait +} + // stopInstance gracefully stops an active instance. // Flow: send Shutdown RPC -> wait for VM to power off -> // fall back to hypervisor shutdown -> final SIGKILL if still alive. diff --git a/lib/instances/vsock.go b/lib/instances/vsock.go index f4060950e..8cbefe4e0 100644 --- a/lib/instances/vsock.go +++ b/lib/instances/vsock.go @@ -14,8 +14,8 @@ func (m *manager) GetVsockDialer(ctx context.Context, instanceID string) (hyperv return nil, err } - if inst.MacOS != nil { - return nil, fmt.Errorf("%w: macOS guest-agent exec/stat is not implemented", ErrInvalidRequest) + if inst.MacOS != nil && !inst.StoredMetadata.GuestAgentEnabled() { + return nil, fmt.Errorf("%w: macOS image does not enable the shared guest agent", ErrInvalidRequest) } return hypervisor.NewVsockDialer(hypervisor.Type(inst.HypervisorType), inst.VsockSocket, inst.VsockCID) } diff --git a/lib/system/guest_agent/darwin_test.go b/lib/system/guest_agent/darwin_test.go new file mode 100644 index 000000000..d7582ea53 --- /dev/null +++ b/lib/system/guest_agent/darwin_test.go @@ -0,0 +1,19 @@ +//go:build darwin + +package main + +import ( + "context" + "testing" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestDarwinNetworkReconfigurationUnsupported(t *testing.T) { + response, err := (&guestServer{}).ReconfigureNetwork(context.Background(), &pb.ReconfigureNetworkRequest{}) + require.Nil(t, response) + require.Equal(t, codes.Unimplemented, status.Code(err)) +} diff --git a/lib/system/guest_agent/exec.go b/lib/system/guest_agent/exec.go index 409754c65..2ebb6e6d3 100644 --- a/lib/system/guest_agent/exec.go +++ b/lib/system/guest_agent/exec.go @@ -2,13 +2,14 @@ package main import ( "context" + "errors" "fmt" - "io" "log" "os" "os/exec" "strings" "sync" + "syscall" "time" "github.com/creack/pty" @@ -30,59 +31,51 @@ func (s *guestServer) Exec(stream pb.GuestService_ExecServer) error { return fmt.Errorf("first message must be ExecStart") } - command := start.Command - if len(command) == 0 { - command = []string{"/bin/sh"} + if len(start.Command) == 0 { + start.Command = []string{"/bin/sh"} } + command := start.Command log.Printf("[guest-agent] exec: command=%v tty=%v cwd=%s timeout=%d", command, start.Tty, start.Cwd, start.TimeoutSeconds) // Create context with timeout if specified - ctx := context.Background() + ctx := stream.Context() if start.TimeoutSeconds > 0 { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, time.Duration(start.TimeoutSeconds)*time.Second) defer cancel() } + ctx, cancel := context.WithCancel(ctx) + defer cancel() + cmd, err := s.execCommand(ctx, start) + if err != nil { + return err + } if start.Tty { - return s.executeTTY(ctx, stream, start) + return s.executeTTY(ctx, cancel, stream, start, cmd) } - return s.executeNoTTY(ctx, stream, start) + return s.executeNoTTY(ctx, cancel, stream, cmd) } // executeNoTTY executes command without TTY -func (s *guestServer) executeNoTTY(ctx context.Context, stream pb.GuestService_ExecServer, start *pb.ExecStart) error { - // Run command directly - guest-agent is already running in container namespace - if len(start.Command) == 0 { - return fmt.Errorf("empty command") - } - - cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) - - // Set up environment (no TTY defaults for non-TTY mode) - cmd.Env = s.buildEnv(start.Env, false) - - // Set up working directory - if start.Cwd != "" { - cmd.Dir = start.Cwd +func (s *guestServer) executeNoTTY(ctx context.Context, cancel context.CancelFunc, stream pb.GuestService_ExecServer, cmd *exec.Cmd) error { + var sendMu sync.Mutex + cmd.Stdout = &execStreamWriter{stream: stream, mu: &sendMu, cancel: cancel} + cmd.Stderr = &execStreamWriter{stream: stream, mu: &sendMu, cancel: cancel, stderr: true} + stdin, err := cmd.StdinPipe() + if err != nil { + return fmt.Errorf("open command stdin: %w", err) } - - stdin, _ := cmd.StdinPipe() - stdout, _ := cmd.StdoutPipe() - stderr, _ := cmd.StderrPipe() - if err := cmd.Start(); err != nil { return fmt.Errorf("start command: %w", err) } - - // Mutex to protect concurrent stream.Send calls (gRPC streams are not thread-safe) - var sendMu sync.Mutex - - // Use WaitGroup to ensure all output is read before sending - var wg sync.WaitGroup - var stdoutData, stderrData []byte + // Cancel is not called once the direct child has exited, so a descendant that + // still holds the pipes would outlive the timeout. Kill the group from the context. + finished := make(chan struct{}) + defer close(finished) + go killGroupOnDone(ctx, cmd, finished) // Handle stdin in background go func() { @@ -98,85 +91,169 @@ func (s *guestServer) executeNoTTY(ctx context.Context, stream pb.GuestService_E } }() - // Read all stdout/stderr BEFORE calling Wait() - Wait() closes the pipes! - wg.Add(1) - go func() { - defer wg.Done() - data, _ := io.ReadAll(stdout) - stdoutData = data - }() - - wg.Add(1) - go func() { - defer wg.Done() - data, _ := io.ReadAll(stderr) - stderrData = data - }() - - // Wait for all reads to complete FIRST (before Wait closes pipes) - wg.Wait() + return s.finishExec(ctx, stream, cmd, nil) +} - // Now safe to call Wait - pipes are fully drained - waitErr := cmd.Wait() +// execCommand owns setup and pre-start cancellation for both I/O modes. +func (s *guestServer) execCommand(ctx context.Context, start *pb.ExecStart) (*exec.Cmd, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("start command: %w", err) + } + cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) + cmd.Cancel = func() error { return killProcessGroup(cmd) } + cmd.WaitDelay = 2 * time.Second + cmd.Env = s.buildEnv(start.Env, start.Tty) + cmd.Dir = start.Cwd + if !start.Tty { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + } + return cmd, nil +} - // Now stream output in chunks (streaming compatible) - const chunkSize = 32 * 1024 - for i := 0; i < len(stdoutData); i += chunkSize { - end := i + chunkSize - if end > len(stdoutData) { - end = len(stdoutData) - } - sendMu.Lock() - stream.Send(&pb.ExecResponse{ - Response: &pb.ExecResponse_Stdout{Stdout: stdoutData[i:end]}, - }) - sendMu.Unlock() +// finishExec owns bounded wait, output drain and final status for both I/O modes. +func (s *guestServer) finishExec(ctx context.Context, stream pb.GuestService_ExecServer, cmd *exec.Cmd, outputDone <-chan struct{}) error { + // Non-TTY cmd.Wait also waits for copies, which can block in stream.Send. + var waitErr error + if err := s.runBounded(ctx, func() { waitErr = cmd.Wait() }); err != nil { + return err } - for i := 0; i < len(stderrData); i += chunkSize { - end := i + chunkSize - if end > len(stderrData) { - end = len(stderrData) + // ErrWaitDelay means the command exited but a descendant still held its output. + // The status is already known, so it is reported as a normal exit. + if waitErr != nil { + _, exited := waitErr.(*exec.ExitError) + if !exited && !errors.Is(waitErr, exec.ErrWaitDelay) { + return fmt.Errorf("stream command output: %w", waitErr) } - sendMu.Lock() - stream.Send(&pb.ExecResponse{ - Response: &pb.ExecResponse_Stderr{Stderr: stderrData[i:end]}, - }) - sendMu.Unlock() } - exitCode := int32(0) - if cmd.ProcessState != nil { - exitCode = int32(cmd.ProcessState.ExitCode()) - } else if waitErr != nil { - // If killed by timeout, exit with 124 (GNU timeout convention) - exitCode = 124 + if outputDone != nil { + if err := awaitBounded(ctx, outputDone, s.drainBound()); err != nil { + return err + } } + exitCode := exitCodeOf(ctx, cmd, waitErr) log.Printf("[guest-agent] command finished with exit code: %d", exitCode) - // Send exit code - return stream.Send(&pb.ExecResponse{ - Response: &pb.ExecResponse_ExitCode{ExitCode: exitCode}, - }) + return s.sendExitCode(ctx, stream, exitCode) } -// executeTTY executes command with TTY -func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_ExecServer, start *pb.ExecStart) error { - // Run command directly with PTY - guest-agent is already running in container namespace - // This ensures PTY and shell are in the same namespace, fixing Ctrl+C signal handling - if len(start.Command) == 0 { - return fmt.Errorf("empty command") +// defaultDrainGrace bounds how long a cancelled command may take to finish. A descendant that +// holds the terminal or output, or a client that stopped reading, must not block the handler. +const defaultDrainGrace = 5 * time.Second + +func (s *guestServer) drainBound() time.Duration { + if s.drainGrace > 0 { + return s.drainGrace } + return defaultDrainGrace +} - cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) +// awaitBounded waits for done. After ctx ends it allows grace more, then returns an +// error so the handler returns, ending the RPC and releasing any send still blocked on it. +func awaitBounded(ctx context.Context, done <-chan struct{}, grace time.Duration) error { + select { + case <-done: + return nil + case <-ctx.Done(): + } + timer := time.NewTimer(grace) + defer timer.Stop() + select { + case <-done: + return nil + case <-timer.C: + return fmt.Errorf("command did not finish after cancellation: %w", ctx.Err()) + } +} + +// killGroupOnDone kills cmd's process group once ctx ends, unless finished closes first. +func killGroupOnDone(ctx context.Context, cmd *exec.Cmd, finished <-chan struct{}) { + select { + case <-ctx.Done(): + _ = killProcessGroup(cmd) + case <-finished: + } +} + +// sendExitCode delivers the final status under the same bound as output, so a client +// that stopped reading cannot hold the handler past cancellation. +func (s *guestServer) sendExitCode(ctx context.Context, stream pb.GuestService_ExecServer, exitCode int32) error { + var err error + if boundErr := s.runBounded(ctx, func() { + err = stream.Send(&pb.ExecResponse{ + Response: &pb.ExecResponse_ExitCode{ExitCode: exitCode}, + }) + }); boundErr != nil { + return boundErr + } + return err +} + +// runBounded runs fn on its own goroutine and waits for it under the drain bound. +// If the bound expires, fn keeps running and must not rely on the handler still waiting. +func (s *guestServer) runBounded(ctx context.Context, fn func()) error { + done := make(chan struct{}) + go func() { + fn() + close(done) + }() + return awaitBounded(ctx, done, s.drainBound()) +} + +// exitCodeOf reports how a command ended. A command that hit its deadline exits 124 +// (GNU timeout convention) even though the group kill leaves a process state behind. +func exitCodeOf(ctx context.Context, cmd *exec.Cmd, waitErr error) int32 { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return 124 + } + if cmd.ProcessState != nil { + return int32(cmd.ProcessState.ExitCode()) + } + if waitErr != nil { + return 124 + } + return 0 +} + +// killProcessGroup kills the command and its descendants. Both exec paths start the command +// as a group leader (Setpgid for no-TTY, Setsid for TTY), so the group id is its pid. +func killProcessGroup(cmd *exec.Cmd) error { + if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { + return err + } + return nil +} - // Set up environment (TTY mode adds TERM default) - cmd.Env = s.buildEnv(start.Env, true) +type execStreamWriter struct { + stream pb.GuestService_ExecServer + mu *sync.Mutex + cancel context.CancelFunc + stderr bool +} - // Set up working directory - if start.Cwd != "" { - cmd.Dir = start.Cwd +func (w *execStreamWriter) Write(data []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + written := 0 + for len(data) > 0 { + n := min(len(data), 32*1024) + response := &pb.ExecResponse{Response: &pb.ExecResponse_Stdout{Stdout: data[:n]}} + if w.stderr { + response.Response = &pb.ExecResponse_Stderr{Stderr: data[:n]} + } + if err := w.stream.Send(response); err != nil { + w.cancel() + return written, err + } + written += n + data = data[n:] } + return written, nil +} + +// executeTTY executes command with TTY +func (s *guestServer) executeTTY(ctx context.Context, cancel context.CancelFunc, stream pb.GuestService_ExecServer, start *pb.ExecStart, cmd *exec.Cmd) error { // Set up initial window size (use defaults if not specified) ws := &pty.Winsize{ @@ -196,12 +273,15 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe return fmt.Errorf("start pty: %w", err) } defer ptmx.Close() + finished := make(chan struct{}) + defer close(finished) + go killGroupOnDone(ctx, cmd, finished) // Mutex to protect concurrent stream.Send calls (gRPC streams are not thread-safe) var sendMu sync.Mutex - // Use WaitGroup to ensure all output is sent before exit code - var wg sync.WaitGroup + outputDone := make(chan struct{}) + output := &execStreamWriter{stream: stream, mu: &sendMu, cancel: cancel} // Handle stdin and resize in background go func() { @@ -226,18 +306,15 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe }() // Stream output - wg.Add(1) go func() { - defer wg.Done() + defer close(outputDone) buf := make([]byte, 32*1024) for { n, err := ptmx.Read(buf) if n > 0 { - sendMu.Lock() - stream.Send(&pb.ExecResponse{ - Response: &pb.ExecResponse_Stdout{Stdout: buf[:n]}, - }) - sendMu.Unlock() + if _, sendErr := output.Write(buf[:n]); sendErr != nil { + return + } } if err != nil { return @@ -245,26 +322,7 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe } }() - // Wait for command or context cancellation - waitErr := cmd.Wait() - - // Wait for all output to be sent - wg.Wait() - - exitCode := int32(0) - if cmd.ProcessState != nil { - exitCode = int32(cmd.ProcessState.ExitCode()) - } else if waitErr != nil { - // If killed by timeout, exit with 124 (GNU timeout convention) - exitCode = 124 - } - - log.Printf("[guest-agent] TTY command finished with exit code: %d", exitCode) - - // Send exit code - return stream.Send(&pb.ExecResponse{ - Response: &pb.ExecResponse_ExitCode{ExitCode: exitCode}, - }) + return s.finishExec(ctx, stream, cmd, outputDone) } // buildEnv constructs environment variables by merging provided env with defaults. diff --git a/lib/system/guest_agent/exec_send_cancel_test.go b/lib/system/guest_agent/exec_send_cancel_test.go new file mode 100644 index 000000000..9419f3447 --- /dev/null +++ b/lib/system/guest_agent/exec_send_cancel_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "errors" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +type failedOutputExecStream struct { + stalledExecStream + failure error +} + +func (s *failedOutputExecStream) Send(*pb.ExecResponse) error { return s.failure } + +func TestExecSendFailureCancelsProcessInBothModes(t *testing.T) { + for _, tty := range []bool{false, true} { + name := "stream" + if tty { + name = "tty" + } + t.Run(name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + release := make(chan struct{}) + defer close(release) + pidFile := filepath.Join(t.TempDir(), "pid") + failure := errors.New("synthetic send failure") + stream := &failedOutputExecStream{stalledExecStream: stalledExecStream{ctx: ctx, release: release, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{Tty: tty, Command: []string{"/bin/sh", "-c", `echo $$ > "$1"; printf ready; exec /bin/sleep 30`, "sh", pidFile}}}}}, failure: failure} + require.Error(t, (&guestServer{}).Exec(stream)) + require.NoError(t, ctx.Err(), "send failure must cancel without waiting for the caller deadline") + data, err := os.ReadFile(pidFile) + require.NoError(t, err) + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + require.NoError(t, err) + require.Eventually(t, func() bool { return errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) }, time.Second, 10*time.Millisecond, "failed output must not leave the command alive") + }) + } +} diff --git a/lib/system/guest_agent/exec_start_cancel_test.go b/lib/system/guest_agent/exec_start_cancel_test.go new file mode 100644 index 000000000..96b5398ae --- /dev/null +++ b/lib/system/guest_agent/exec_start_cancel_test.go @@ -0,0 +1,23 @@ +package main + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +func TestCancelledExecDoesNotAttemptStart(t *testing.T) { + for _, tty := range []bool{false, true} { + t.Run(fmt.Sprintf("tty=%t", tty), func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + stream := &stalledExecStream{ctx: ctx, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{Command: []string{filepath.Join(t.TempDir(), "nonexistent-executable")}, Tty: tty}}}} + err := (&guestServer{}).Exec(stream) + require.ErrorIs(t, err, context.Canceled, "a canceled request must be rejected before trying to start the command") + }) + } +} diff --git a/lib/system/guest_agent/exec_test.go b/lib/system/guest_agent/exec_test.go index ca3440536..887a72d7b 100644 --- a/lib/system/guest_agent/exec_test.go +++ b/lib/system/guest_agent/exec_test.go @@ -16,9 +16,9 @@ func TestBuildEnv(t *testing.T) { }) t.Run("non-TTY session does not add xterm-256color", func(t *testing.T) { + t.Setenv("TERM", "guest-agent-test") env := s.buildEnv(nil, false) - // Non-TTY should not add our default TERM - // (host environment TERM may still be present, that's fine) + assert.Contains(t, env, "TERM=guest-agent-test", "non-TTY preserves the inherited TERM") assert.NotContains(t, env, "TERM=xterm-256color", "non-TTY should not add xterm-256color default") }) diff --git a/lib/system/guest_agent/main.go b/lib/system/guest_agent/main.go index cc8f5d049..5f8159fc6 100644 --- a/lib/system/guest_agent/main.go +++ b/lib/system/guest_agent/main.go @@ -3,35 +3,36 @@ package main import ( "fmt" "log" + "net" "os" "path/filepath" "strconv" "time" pb "github.com/kernel/hypeman/lib/guest" - "github.com/mdlayher/vsock" "google.golang.org/grpc" ) const ( - readySentinelPrefix = "HYPEMAN-AGENT-READY" - defaultReadyFilePath = "/run/hypeman/guest-agent-ready" - readyFDEnv = "HYPEMAN_AGENT_READY_FD" + readySentinelPrefix = "HYPEMAN-AGENT-READY" + readyFDEnv = "HYPEMAN_AGENT_READY_FD" ) // guestServer implements the gRPC GuestService type guestServer struct { pb.UnimplementedGuestServiceServer gpuReporter *gpuInitReporter + // drainGrace bounds how long a cancelled exec may take to finish; zero means defaultDrainGrace. + drainGrace time.Duration } func main() { // Listen on vsock port 2222 with retries - var l *vsock.Listener + var l net.Listener var err error for i := 0; i < 10; i++ { - l, err = vsock.Listen(2222, nil) + l, err = listenVsock(2222) if err == nil { break } diff --git a/lib/system/guest_agent/network.go b/lib/system/guest_agent/network.go index 66b71bad8..3b01f5e53 100644 --- a/lib/system/guest_agent/network.go +++ b/lib/system/guest_agent/network.go @@ -1,3 +1,5 @@ +//go:build linux + package main import ( diff --git a/lib/system/guest_agent/network_darwin.go b/lib/system/guest_agent/network_darwin.go new file mode 100644 index 000000000..38a2370ce --- /dev/null +++ b/lib/system/guest_agent/network_darwin.go @@ -0,0 +1,15 @@ +//go:build darwin + +package main + +import ( + "context" + + pb "github.com/kernel/hypeman/lib/guest" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func (s *guestServer) ReconfigureNetwork(context.Context, *pb.ReconfigureNetworkRequest) (*pb.ReconfigureNetworkResponse, error) { + return nil, status.Error(codes.Unimplemented, "Darwin network identity reconfiguration is not supported; VZ NAT uses guest DHCP") +} diff --git a/lib/system/guest_agent/ready_darwin.go b/lib/system/guest_agent/ready_darwin.go new file mode 100644 index 000000000..5504a5ae8 --- /dev/null +++ b/lib/system/guest_agent/ready_darwin.go @@ -0,0 +1,7 @@ +//go:build darwin + +package main + +// defaultReadyFilePath is where the guest agent records readiness when +// HYPEMAN_AGENT_READY_FILE is unset. +const defaultReadyFilePath = "/var/run/hypeman/guest-agent-ready" diff --git a/lib/system/guest_agent/ready_linux.go b/lib/system/guest_agent/ready_linux.go new file mode 100644 index 000000000..724c7c0fc --- /dev/null +++ b/lib/system/guest_agent/ready_linux.go @@ -0,0 +1,7 @@ +//go:build linux + +package main + +// defaultReadyFilePath is where the guest agent records readiness when +// HYPEMAN_AGENT_READY_FILE is unset. +const defaultReadyFilePath = "/run/hypeman/guest-agent-ready" diff --git a/lib/system/guest_agent/service_test.go b/lib/system/guest_agent/service_test.go new file mode 100644 index 000000000..b925dfa4a --- /dev/null +++ b/lib/system/guest_agent/service_test.go @@ -0,0 +1,356 @@ +package main + +import ( + "context" + "errors" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/test/bufconn" +) + +// Exercise the shared RPC contract without starting a VM or listening on vsock. +func testGuestClient(t *testing.T) pb.GuestServiceClient { + t.Helper() + listener := bufconn.Listen(64 << 10) + server := grpc.NewServer() + pb.RegisterGuestServiceServer(server, &guestServer{}) + go server.Serve(listener) + t.Cleanup(server.Stop) + t.Cleanup(func() { listener.Close() }) + conn, err := grpc.NewClient("passthrough:///guest-test", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { + return listener.Dial() + })) + require.NoError(t, err) + t.Cleanup(func() { conn.Close() }) + return pb.NewGuestServiceClient(conn) +} + +func TestGuestServiceExecRoundTrip(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "printf '%s' \"$HYPEMAN_EXEC_TEST\"; printf 'stderr' >&2; exit 7"}, + Env: map[string]string{"HYPEMAN_EXEC_TEST": "stdout"}, + }}})) + require.NoError(t, stream.CloseSend()) + var stdout, stderr strings.Builder + var exitCode *int32 + for { + response, err := stream.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + switch value := response.Response.(type) { + case *pb.ExecResponse_Stdout: + stdout.Write(value.Stdout) + case *pb.ExecResponse_Stderr: + stderr.Write(value.Stderr) + case *pb.ExecResponse_ExitCode: + code := value.ExitCode + exitCode = &code + } + } + require.Equal(t, "stdout", stdout.String()) + require.Equal(t, "stderr", stderr.String()) + require.NotNil(t, exitCode) + require.Equal(t, int32(7), *exitCode) +} + +func TestGuestServiceExecStreamsBeforeExit(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "printf ready; read input; printf '%s' \"$input\""}, + }}})) + first, err := stream.Recv() + require.NoError(t, err, "output must arrive while the command is still waiting for stdin") + require.Equal(t, "ready", string(first.GetStdout())) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Stdin{Stdin: []byte("finish\n")}})) + require.NoError(t, stream.CloseSend()) + var output strings.Builder + exited := false + for { + response, err := stream.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + output.Write(response.GetStdout()) + if code, ok := response.Response.(*pb.ExecResponse_ExitCode); ok { + require.Equal(t, int32(0), code.ExitCode) + exited = true + } + } + require.True(t, exited) + require.Equal(t, "finish", output.String()) +} + +func TestGuestServiceExecCancellation(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pidPath := filepath.Join(t.TempDir(), "pid") + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "printf '%s' \"$$\" > \"$HYPEMAN_PID_FILE\"; exec /bin/sleep 30"}, + Env: map[string]string{"HYPEMAN_PID_FILE": pidPath}, + }}})) + require.NoError(t, stream.CloseSend()) + var pid int + require.Eventually(t, func() bool { + data, err := os.ReadFile(pidPath) + if err != nil { + return false + } + pid, err = strconv.Atoi(string(data)) + return err == nil && pid > 0 + }, 5*time.Second, 10*time.Millisecond) + defer syscall.Kill(pid, syscall.SIGKILL) + cancel() + require.Eventually(t, func() bool { + return syscall.Kill(pid, 0) == syscall.ESRCH + }, 5*time.Second, 10*time.Millisecond, "disconnect must cancel the command, not leave it running") +} + +func TestGuestServiceExecCancellationKillsDescendants(t *testing.T) { + for _, tty := range []bool{false, true} { + t.Run(map[bool]string{false: "pipes", true: "tty"}[tty], func(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pidPath := filepath.Join(t.TempDir(), "pid") + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "sleep 30 & printf '%s' \"$!\" > \"$HYPEMAN_PID_FILE\"; wait"}, + Env: map[string]string{"HYPEMAN_PID_FILE": pidPath}, + Tty: tty, + }}})) + require.NoError(t, stream.CloseSend()) + var pid int + require.Eventually(t, func() bool { + data, err := os.ReadFile(pidPath) + if err != nil { + return false + } + pid, err = strconv.Atoi(string(data)) + return err == nil && pid > 0 + }, 5*time.Second, 10*time.Millisecond) + defer syscall.Kill(pid, syscall.SIGKILL) + cancel() + require.Eventually(t, func() bool { + return processGone(pid) + }, 5*time.Second, 10*time.Millisecond, "cancellation must reach descendants, not only the shell") + }) + } +} + +// processGone reports whether pid no longer runs. A zombie counts as gone. +func processGone(pid int) bool { + if errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) { + return true + } + out, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() + return err != nil || strings.HasPrefix(strings.TrimSpace(string(out)), "Z") +} + +// stalledExecStream models a client that stopped reading: Send blocks until release closes. +// With stallExitOnly, only the final exit-code send blocks. +type stalledExecStream struct { + grpc.ServerStream + ctx context.Context + start *pb.ExecRequest + release chan struct{} + stallExitOnly bool +} + +func (s *stalledExecStream) Context() context.Context { return s.ctx } +func (s *stalledExecStream) Recv() (*pb.ExecRequest, error) { + if s.start != nil { + req := s.start + s.start = nil + return req, nil + } + <-s.ctx.Done() + return nil, s.ctx.Err() +} +func (s *stalledExecStream) Send(resp *pb.ExecResponse) error { + if _, exit := resp.Response.(*pb.ExecResponse_ExitCode); s.stallExitOnly && !exit { + return nil + } + <-s.release + return nil +} + +// runStalledExec runs a timed-out command against a client that stopped reading and +// requires the handler to return the bounded-drain error instead of hanging. +func runStalledExec(t *testing.T, command string, exitOnly bool) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + stream := &stalledExecStream{ctx: ctx, release: release, stallExitOnly: exitOnly, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", command}, + TimeoutSeconds: 1, + }}}} + done := make(chan error, 1) + go func() { done <- (&guestServer{drainGrace: 100 * time.Millisecond}).Exec(stream) }() + select { + case err := <-done: + require.ErrorContains(t, err, "did not finish") + case <-time.After(10 * time.Second): + t.Fatal("exec stayed blocked behind a stalled client after its timeout") + } +} + +func TestGuestServiceExecTimeoutFinishesWhenClientStopsReading(t *testing.T) { + runStalledExec(t, "yes", false) +} + +func TestGuestServiceExecTimeoutBoundsExitCodeSend(t *testing.T) { + runStalledExec(t, "echo started; sleep 30", true) +} + +func TestGuestServiceExecTimeoutKillsDescendantAfterShellExits(t *testing.T) { + for _, tty := range []bool{false, true} { + t.Run(map[bool]string{false: "pipes", true: "tty"}[tty], func(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pidPath := filepath.Join(t.TempDir(), "pid") + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "sleep 30 & printf '%s' \"$!\" > \"$HYPEMAN_PID_FILE\"; exit 0"}, + Env: map[string]string{"HYPEMAN_PID_FILE": pidPath}, + Tty: tty, + TimeoutSeconds: 1, + }}})) + require.NoError(t, stream.CloseSend()) + var pid int + require.Eventually(t, func() bool { + data, err := os.ReadFile(pidPath) + if err != nil { + return false + } + pid, err = strconv.Atoi(string(data)) + return err == nil && pid > 0 + }, 5*time.Second, 10*time.Millisecond) + defer syscall.Kill(pid, syscall.SIGKILL) + finished := make(chan error, 1) + go func() { + for { + if _, err := stream.Recv(); err != nil { + finished <- err + return + } + } + }() + select { + case <-finished: + case <-time.After(10 * time.Second): + t.Fatal("RPC stayed open while a descendant held the command's output") + } + require.True(t, processGone(pid), "timeout must kill the descendant even after the shell exited") + }) + } +} + +// runExec drives one exec to completion and returns its stdout and exit code. +func runExec(t *testing.T, start *pb.ExecStart) (string, int32) { + t.Helper() + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: start}})) + require.NoError(t, stream.CloseSend()) + var stdout strings.Builder + exit := int32(-1) + for { + resp, err := stream.Recv() + if errors.Is(err, io.EOF) { + break + } + require.NoError(t, err) + stdout.Write(resp.GetStdout()) + if code, ok := resp.Response.(*pb.ExecResponse_ExitCode); ok { + exit = code.ExitCode + } + } + return stdout.String(), exit +} + +func TestGuestServiceExecExitZeroWhenDescendantHoldsOutput(t *testing.T) { + stdout, exit := runExec(t, &pb.ExecStart{Command: []string{"/bin/sh", "-c", "sleep 5 & echo hi"}}) + require.Equal(t, "hi\n", stdout) + require.Equal(t, int32(0), exit, "a command that exits 0 must report 0, not a stream error") +} + +func TestGuestServiceExecTimeoutReports124(t *testing.T) { + _, exit := runExec(t, &pb.ExecStart{Command: []string{"/bin/sleep", "30"}, TimeoutSeconds: 1}) + require.Equal(t, int32(124), exit) +} + +func TestGuestServiceFileRoundTrip(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + path := filepath.Join(t.TempDir(), "copied") + payload := "shared guest service file payload" + stream, err := client.CopyToGuest(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.CopyToGuestRequest{Request: &pb.CopyToGuestRequest_Start{Start: &pb.CopyToGuestStart{Path: path, Mode: 0600, Size: int64(len(payload))}}})) + require.NoError(t, stream.Send(&pb.CopyToGuestRequest{Request: &pb.CopyToGuestRequest_Data{Data: []byte(payload)}})) + require.NoError(t, stream.Send(&pb.CopyToGuestRequest{Request: &pb.CopyToGuestRequest_End{End: &pb.CopyToGuestEnd{}}})) + result, err := stream.CloseAndRecv() + require.NoError(t, err) + require.True(t, result.Success, result.Error) + require.Equal(t, int64(len(payload)), result.BytesWritten) + info, err := client.StatPath(ctx, &pb.StatPathRequest{Path: path}) + require.NoError(t, err) + require.True(t, info.Exists && info.IsFile) + require.Equal(t, uint32(0600), info.Mode&0777) + require.Equal(t, int64(len(payload)), info.Size) + read, err := client.CopyFromGuest(ctx, &pb.CopyFromGuestRequest{Path: path}) + require.NoError(t, err) + var content strings.Builder + final := false + for { + response, err := read.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + require.Nil(t, response.GetError()) + content.Write(response.GetData()) + if response.GetEnd() != nil && response.GetEnd().Final { + final = true + } + } + require.True(t, final) + require.Equal(t, payload, content.String()) +} diff --git a/lib/system/guest_agent/shutdown.go b/lib/system/guest_agent/shutdown.go index e29690aef..5eed220cb 100644 --- a/lib/system/guest_agent/shutdown.go +++ b/lib/system/guest_agent/shutdown.go @@ -1,3 +1,5 @@ +//go:build linux + package main import ( diff --git a/lib/system/guest_agent/shutdown_darwin.go b/lib/system/guest_agent/shutdown_darwin.go new file mode 100644 index 000000000..fcca21846 --- /dev/null +++ b/lib/system/guest_agent/shutdown_darwin.go @@ -0,0 +1,41 @@ +//go:build darwin + +package main + +import ( + "context" + "log" + "os" + "os/exec" + "syscall" + + pb "github.com/kernel/hypeman/lib/guest" +) + +// Darwin's launchd is not Linux init: sending it SIGTERM is not a shutdown API. +func (s *guestServer) Shutdown(ctx context.Context, req *pb.ShutdownRequest) (*pb.ShutdownResponse, error) { + return requestDarwinShutdown(ctx, req, os.Geteuid(), func(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return err + } + command := darwinShutdownCommand() + if err := command.Start(); err != nil { + return err + } + go func() { + if err := command.Wait(); err != nil { + log.Printf("scheduled Darwin shutdown failed: %v", err) + } + }() + return nil + }) +} + +func darwinShutdownCommand() *exec.Cmd { + // A short detached delay lets gRPC send the acceptance reply before macOS + // tears down vsock. The host must still independently confirm owned VMM exit. + // Once accepted, shutdown is not cancelled by the RPC connection disappearing. + command := exec.Command("/bin/sh", "-c", "/bin/sleep 1; exec /sbin/shutdown -h now") + command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + return command +} diff --git a/lib/system/guest_agent/shutdown_policy.go b/lib/system/guest_agent/shutdown_policy.go new file mode 100644 index 000000000..710a430a0 --- /dev/null +++ b/lib/system/guest_agent/shutdown_policy.go @@ -0,0 +1,28 @@ +package main + +import ( + "context" + "syscall" + + pb "github.com/kernel/hypeman/lib/guest" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// requestDarwinShutdown applies the Darwin shutdown policy. It takes the effective +// uid and the shutdown command as arguments, so the policy runs on every host. +func requestDarwinShutdown(ctx context.Context, req *pb.ShutdownRequest, euid int, run func(context.Context) error) (*pb.ShutdownResponse, error) { + if req.Signal != 0 && req.Signal != int32(syscall.SIGTERM) { + return nil, status.Error(codes.InvalidArgument, "Darwin supports only an orderly shutdown, not arbitrary init signals") + } + if euid != 0 { + return nil, status.Error(codes.PermissionDenied, "Darwin shutdown requires the system guest agent running as root") + } + if err := ctx.Err(); err != nil { + return nil, status.FromContextError(err).Err() + } + if err := run(ctx); err != nil { + return nil, status.Errorf(codes.Internal, "Darwin shutdown failed: %v", err) + } + return &pb.ShutdownResponse{}, nil +} diff --git a/lib/system/guest_agent/shutdown_policy_test.go b/lib/system/guest_agent/shutdown_policy_test.go new file mode 100644 index 000000000..c2c5873cd --- /dev/null +++ b/lib/system/guest_agent/shutdown_policy_test.go @@ -0,0 +1,52 @@ +package main + +import ( + "context" + "errors" + "syscall" + "testing" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestDarwinShutdownPolicy(t *testing.T) { + for _, tc := range []struct { + name string + signal int32 + euid int + canceled bool + commandError error + code codes.Code + called bool + }{ + {name: "default orderly shutdown", called: true}, + {name: "explicit orderly shutdown", signal: int32(syscall.SIGTERM), called: true}, + {name: "reject arbitrary signal", signal: int32(syscall.SIGKILL), code: codes.InvalidArgument}, + {name: "reject desktop agent", euid: 501, code: codes.PermissionDenied}, + {name: "canceled before command", canceled: true, code: codes.Canceled}, + {name: "command failure", commandError: errors.New("shutdown refused"), code: codes.Internal, called: true}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + if tc.canceled { + cancel() + } + called := false + response, err := requestDarwinShutdown(ctx, &pb.ShutdownRequest{Signal: tc.signal}, tc.euid, func(context.Context) error { + called = true + return tc.commandError + }) + require.Equal(t, tc.called, called) + require.Equal(t, tc.code, status.Code(err)) + if tc.code == codes.OK { + require.NotNil(t, response) + } else { + require.Nil(t, response) + } + }) + } +} diff --git a/lib/system/guest_agent/shutdown_schedule_darwin_test.go b/lib/system/guest_agent/shutdown_schedule_darwin_test.go new file mode 100644 index 000000000..017c5d352 --- /dev/null +++ b/lib/system/guest_agent/shutdown_schedule_darwin_test.go @@ -0,0 +1,19 @@ +//go:build darwin + +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDarwinShutdownDefersPowerOffUntilReply(t *testing.T) { + // Inspect only: never execute a host shutdown command in a test. + command := darwinShutdownCommand() + require.Equal(t, "/bin/sh", command.Path) + require.Equal(t, []string{"/bin/sh", "-c", "/bin/sleep 1; exec /sbin/shutdown -h now"}, command.Args) + require.NotNil(t, command.SysProcAttr) + require.True(t, command.SysProcAttr.Setpgid) + require.Nil(t, command.Cancel, "accepted shutdown must survive RPC cancellation") +} diff --git a/lib/system/guest_agent/vsock_darwin.go b/lib/system/guest_agent/vsock_darwin.go new file mode 100644 index 000000000..048267d24 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin.go @@ -0,0 +1,122 @@ +//go:build darwin && cgo + +package main + +/* +#include +#include +#include +#include +#include +static int listen_vm(unsigned port) { + int fd=socket(AF_VSOCK,SOCK_STREAM,0);if(fd<0)return -1; + struct sockaddr_vm a={0};a.svm_len=sizeof(a);a.svm_family=AF_VSOCK;a.svm_cid=VMADDR_CID_ANY;a.svm_port=port; + if(bind(fd,(struct sockaddr*)&a,sizeof(a))<0 || listen(fd,16)<0 || fcntl(fd,F_SETFL,O_NONBLOCK)<0){int e=errno;close(fd);errno=e;return -1;} + if(fcntl(fd,F_SETFD,FD_CLOEXEC)<0){int e=errno;close(fd);errno=e;return -1;}return fd; +} +static int accept_vm(int fd) { + struct sockaddr_vm a={0};socklen_t n=sizeof(a);int c=accept(fd,(struct sockaddr*)&a,&n);if(c<0)return -1; + if(a.svm_cid!=VMADDR_CID_HOST){close(c);errno=EAGAIN;return -1;} + if(fcntl(c,F_SETFL,O_NONBLOCK)<0){int e=errno;close(c);errno=e;return -1;} + if(fcntl(c,F_SETFD,FD_CLOEXEC)<0){int e=errno;close(c);errno=e;return -1;}return c; +} +*/ +import "C" +import ( + "errors" + "fmt" + "net" + "os" + "sync" + "syscall" + "time" +) + +type vmAddr string + +func (a vmAddr) Network() string { return "vsock" } +func (a vmAddr) String() string { return string(a) } + +// Do not promote os.File's zero-copy methods: AF_VSOCK is not a TCP/file +// descriptor and must use ordinary reads/writes for gRPC transport. +type vmConn struct { + file *os.File + local net.Addr +} + +func (c *vmConn) Read(p []byte) (int, error) { + n, err := c.file.Read(p) + if err != nil { + return n, &net.OpError{Op: "read", Net: "vsock", Err: err} + } + return n, err +} +func (c *vmConn) Write(p []byte) (int, error) { + n, err := c.file.Write(p) + if err != nil { + return n, &net.OpError{Op: "write", Net: "vsock", Err: err} + } + return n, err +} +func (c *vmConn) Close() error { return c.file.Close() } +func (c *vmConn) SetDeadline(t time.Time) error { return c.file.SetDeadline(t) } +func (c *vmConn) SetReadDeadline(t time.Time) error { return c.file.SetReadDeadline(t) } +func (c *vmConn) SetWriteDeadline(t time.Time) error { return c.file.SetWriteDeadline(t) } + +func (c *vmConn) LocalAddr() net.Addr { return c.local } +func (c *vmConn) RemoteAddr() net.Addr { return vmAddr("host:2") } + +type vmListener struct { + mu sync.Mutex + fd C.int + port uint32 + closed bool +} + +func listenVsock(port uint32) (net.Listener, error) { + fd, err := C.listen_vm(C.uint(port)) + if fd < 0 { + return nil, fmt.Errorf("listen vsock: %w", err) + } + return &vmListener{fd: fd, port: port}, nil +} +func (l *vmListener) Addr() net.Addr { return vmAddr(fmt.Sprintf("guest:%d", l.port)) } +func (l *vmListener) Close() error { + l.mu.Lock() + defer l.mu.Unlock() + if l.closed { + return net.ErrClosed + } + l.closed = true + C.close(l.fd) + return nil +} +func (l *vmListener) Accept() (net.Conn, error) { + for { + l.mu.Lock() + if l.closed { + l.mu.Unlock() + return nil, net.ErrClosed + } + // Darwin has no accept4: the accepted fd is inheritable until accept_vm sets FD_CLOEXEC. + // Holding the fork lock across that window keeps a concurrent fork from copying it. + syscall.ForkLock.RLock() + fd, err := C.accept_vm(l.fd) + syscall.ForkLock.RUnlock() + l.mu.Unlock() + if fd >= 0 { + return &vmConn{file: os.NewFile(uintptr(fd), "guest-vsock"), local: l.Addr()}, nil + } + if !errors.Is(err, syscall.EAGAIN) && !errors.Is(err, syscall.EINTR) { + return nil, acceptError(err) + } + time.Sleep(10 * time.Millisecond) + } +} + +// acceptError keeps the net.OpError Temporary contract. grpc-go retries only a +// direct Temporary() assertion, so wrapping with fmt.Errorf would stop the server +// on transient EMFILE/ENFILE/ECONNABORTED. +func acceptError(err error) error { + return &net.OpError{Op: "accept", Net: "vsock", Err: err} +} diff --git a/lib/system/guest_agent/vsock_darwin_nocgo.go b/lib/system/guest_agent/vsock_darwin_nocgo.go new file mode 100644 index 000000000..7fae1f3b7 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin_nocgo.go @@ -0,0 +1,12 @@ +//go:build darwin && !cgo + +package main + +import ( + "fmt" + "net" +) + +func listenVsock(uint32) (net.Listener, error) { + return nil, fmt.Errorf("Darwin guest vsock requires a cgo-enabled build") +} diff --git a/lib/system/guest_agent/vsock_darwin_nocgo_test.go b/lib/system/guest_agent/vsock_darwin_nocgo_test.go new file mode 100644 index 000000000..630f8b5d8 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin_nocgo_test.go @@ -0,0 +1,15 @@ +//go:build darwin && !cgo + +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDarwinVsockRequiresCGO(t *testing.T) { + listener, err := listenVsock(2222) + require.Nil(t, listener) + require.ErrorContains(t, err, "cgo-enabled build") +} diff --git a/lib/system/guest_agent/vsock_darwin_test.go b/lib/system/guest_agent/vsock_darwin_test.go new file mode 100644 index 000000000..4fc2829f4 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin_test.go @@ -0,0 +1,48 @@ +//go:build darwin && cgo + +package main + +import ( + "errors" + "net" + "os" + "syscall" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +func TestVsockReadTimeoutImplementsNetError(t *testing.T) { + fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0) + if err != nil { + t.Fatal(err) + } + defer unix.Close(fds[1]) + if err = unix.SetNonblock(fds[0], true); err != nil { + unix.Close(fds[0]) + t.Fatal(err) + } + conn := &vmConn{file: os.NewFile(uintptr(fds[0]), "test-vsock"), local: vmAddr("guest:2222")} + defer conn.Close() + if err = conn.SetReadDeadline(time.Now().Add(-time.Second)); err != nil { + t.Fatal(err) + } + _, err = conn.Read(make([]byte, 1)) + var ne net.Error + if !errors.As(err, &ne) || !ne.Timeout() { + t.Fatalf("transport timeout must implement net.Error: %v", err) + } +} + +func TestVsockAcceptErrorsRemainGRPCTemporary(t *testing.T) { + for _, errno := range []syscall.Errno{syscall.EMFILE, syscall.ENFILE, syscall.ECONNABORTED} { + err := acceptError(errno) + if ne, ok := err.(interface{ Temporary() bool }); !ok || !ne.Temporary() { + t.Fatalf("%v: grpc direct Temporary assertion failed for %T", errno, err) + } + if !errors.Is(err, errno) { + t.Fatalf("%v: lost errno", errno) + } + } +} diff --git a/lib/system/guest_agent/vsock_linux.go b/lib/system/guest_agent/vsock_linux.go new file mode 100644 index 000000000..3de52ddc5 --- /dev/null +++ b/lib/system/guest_agent/vsock_linux.go @@ -0,0 +1,13 @@ +//go:build linux + +package main + +import ( + "net" + + "github.com/mdlayher/vsock" +) + +func listenVsock(port uint32) (net.Listener, error) { + return vsock.Listen(port, nil) +}