fix(agent): rebuild non-persisting guest-agent handoff

api/v1alpha1/impvm_types.go

@@ -224,6 +224,21 @@ type ImpVMStatus struct {
 	// +optional
 	CiliumEndpointID int64 `json:"ciliumEndpointID,omitempty"`
 
+	// RunnerHandoffAccepted records that the one-time runner payload was handed
+	// to the guest Exec boundary. It is also the durable ambiguity marker: an
+	// agent restart must not replay a payload after this is set.
+	// +optional
+	RunnerHandoffAccepted bool `json:"runnerHandoffAccepted,omitempty"`
+
+	// RunnerExitCode is the non-secret exit result returned by the guest runner.
+	// +optional
+	RunnerExitCode *int32 `json:"runnerExitCode,omitempty"`
+
+	// RunnerFailureReason is a bounded, non-secret handoff failure summary.
+	// +optional
+	// +kubebuilder:validation:MaxLength=256
+	RunnerFailureReason string `json:"runnerFailureReason,omitempty"`
+
 	// Conditions follow the standard k8s condition convention.
 	// +optional
 	// +listType=map

config/crd/bases/imp.dev_impvms.yaml

@@ -789,6 +789,22 @@ spec:
                   has been restarted.
                 format: int32
                 type: integer
+              runnerExitCode:
+                description: RunnerExitCode is the non-secret exit result returned
+                  by the guest runner.
+                format: int32
+                type: integer
+              runnerFailureReason:
+                description: RunnerFailureReason is a bounded, non-secret handoff
+                  failure summary.
+                maxLength: 256
+                type: string
+              runnerHandoffAccepted:
+                description: |-
+                  RunnerHandoffAccepted records that the one-time runner payload was handed
+                  to the guest Exec boundary. It is also the durable ambiguity marker: an
+                  agent restart must not replay a payload after this is set.
+                type: boolean
               runningAt:
                 description: RunningAt is the time the VM first reached Running phase.
                 format: date-time

internal/agent/runnerlaunch/launcher.go

@@ -7,9 +7,8 @@
 // referenced by ImpVM.spec.runnerConfigSecret. After the VM reaches Running,
 // the agent dials the guest agent over VSOCK, sends an Exec for
 // /usr/local/bin/runner with IMP_GITHUB_JITCONFIG in the command environment,
-// and deletes the Secret — the payload is one-time and must not outlive
-// handoff. If the Secret is already gone (agent restart after a successful
-// handoff) the launcher is a no-op.
+// and deletes the Secret only after the runner exits. The payload is one-time
+// and must not be replayed after an ambiguous handoff.
 package runnerlaunch
 
 import (
@@ -40,6 +39,19 @@ var (
 )
 
 const maxRunnerStderrLogBytes = 1024
+const maxRunnerFailureReasonBytes = 256
+
+func boundedFailureReason(reason, payload string) string {
+	reason = sanitizeRunnerStderr(reason, payload)
+	if len(reason) <= maxRunnerFailureReasonBytes {
+		return reason
+	}
+	limit := maxRunnerFailureReasonBytes - 3
+	for limit > 0 && (reason[limit]&0xc0) == 0x80 {
+		limit--
+	}
+	return reason[:limit] + "..."
+}
 
 // sanitizeRunnerStderr keeps enough guest diagnostics to identify startup
 // failures without copying one-time credentials into controller logs.
@@ -127,14 +139,23 @@ func (l *Launcher) Run(ctx context.Context, vm *impv1alpha1.ImpVM) Result {
 	if secretName == "" {
 		return Result{}
 	}
-	launchKey := vm.Namespace + "/" + vm.Name
+	launchKey := vm.Namespace + "/" + vm.Name + "/" + string(vm.UID)
 	if _, loaded := launches.LoadOrStore(launchKey, struct{}{}); loaded {
 		return Result{}
 	}
 	defer launches.Delete(launchKey)
 
-	// Read the payload. Missing Secret after an agent restart means the
-	// handoff already happened; treat as done.
+	// The accepted marker is written before Exec because the unary RPC has no
+	// acknowledgement boundary: it returns only when the guest process exits.
+	// A restart after this write is therefore ambiguous and must not replay.
+	if vm.Status.RunnerExitCode != nil || vm.Status.RunnerHandoffAccepted {
+		if vm.Status.RunnerExitCode != nil {
+			return Result{HandoffDone: true, ExitCode: *vm.Status.RunnerExitCode}
+		}
+		return Result{Err: fmt.Errorf("runner handoff for VM UID %s is ambiguous; refusing replay", vm.UID)}
+	}
+
+	// Read the payload through the uncached APIReader by exact namespace/name.
 	var secret corev1.Secret
 	reader := l.Reader
 	if reader == nil {
@@ -144,11 +165,10 @@ func (l *Launcher) Run(ctx context.Context, vm *impv1alpha1.ImpVM) Result {
 		return Result{Err: fmt.Errorf("runner launcher requires Kubernetes reader and client")}
 	}
 	err := reader.Get(ctx, client.ObjectKey{Namespace: vm.Namespace, Name: secretName}, &secret)
-	switch {
-	case apierrors.IsNotFound(err):
-		log.Info("runner config Secret already deleted; handoff previously completed", "secret", secretName)
-		return Result{HandoffDone: true}
-	case err != nil:
+	if err != nil {
+		l.recordFailure(ctx, vm, err.Error(), "")
+		// NotFound is intentionally not interpreted as a completed handoff: it
+		// may be a cleanup race or an agent restart in an unknown state.
 		return Result{Err: fmt.Errorf("get runner config Secret %s: %w", secretName, err)}
 	}
 	config, err := runner.JITConfigFromSecretValue(
@@ -157,18 +177,24 @@ func (l *Launcher) Run(ctx context.Context, vm *impv1alpha1.ImpVM) Result {
 		secret.Annotations[runner.JITConfigRunnerAnnotation],
 	)
 	if err != nil {
+		l.recordFailure(ctx, vm, err.Error(), "")
 		return Result{Err: fmt.Errorf("decode runner config Secret %s: %w", secretName, err)}
 	}
 
+	if err := l.recordAccepted(ctx, vm); err != nil {
+		return Result{Err: fmt.Errorf("record runner handoff acceptance: %w", err)}
+	}
+
 	// Dial the guest agent, retrying while the guest finishes booting.
 	conn, err := l.dialWithRetry(ctx)
 	if err != nil {
+		l.recordFailure(ctx, vm, err.Error(), config.EncodedConfig)
 		return Result{Err: fmt.Errorf("dial guest agent: %w", err)}
 	}
 	defer conn.Close() //nolint:errcheck
 	guest := pb.NewGuestAgentClient(conn)
 
-	// Hand off. env keeps the payload out of argv (not visible in guest ps).
+	// Hand off. Env keeps the payload out of argv (not visible in guest ps).
 	hCtx, cancel := context.WithTimeout(ctx, handoffTimeout)
 	defer cancel()
 	resp, err := guest.Exec(hCtx, &pb.ExecRequest{
@@ -180,22 +206,27 @@ func (l *Launcher) Run(ctx context.Context, vm *impv1alpha1.ImpVM) Result {
 		TimeoutSeconds: int32(handoffTimeout / time.Second),
 	})
 	if err != nil {
+		l.recordFailure(ctx, vm, err.Error(), config.EncodedConfig)
 		return Result{Err: fmt.Errorf("launch runner in guest: %w", err)}
 	}
 
-	// One-time payload: delete immediately after successful delivery.
-	delErr := l.Client.Delete(ctx, &secret)
-	if delErr != nil && !apierrors.IsNotFound(delErr) {
-		// Non-fatal: expiry and pool cleanup will collect it, but surface it.
-		log.Error("failed to delete runner config Secret after handoff",
-			"secret", secretName, "err", delErr)
-	}
-
 	if resp.ExitCode != 0 {
 		if stderr := runnerFailureStderr(resp.ExitCode, resp.Stderr, config.EncodedConfig); stderr != "" {
 			log.Warn("runner emitted stderr", "stderr", stderr)
 		}
 	}
+	if err := l.recordOutcome(ctx, vm, resp.ExitCode, runnerFailureStderr(resp.ExitCode, resp.Stderr, config.EncodedConfig)); err != nil {
+		log.Error("failed to persist runner handoff outcome", "err", err)
+	}
+
+	// The Secret remains until unary Exec returns, which is the runner-exit
+	// boundary. Delete-NotFound is an idempotent cleanup success.
+	delErr := l.Client.Delete(ctx, &secret)
+	if delErr != nil && !apierrors.IsNotFound(delErr) {
+		log.Error("failed to delete runner config Secret after runner exit",
+			"secret", secretName, "err", delErr)
+	}
+
 	log.Info("runner exited", "exit_code", resp.ExitCode, "secret", secretName)
 	if resp.ExitCode != 0 {
 		return Result{HandoffDone: true, ExitCode: resp.ExitCode,
@@ -204,6 +235,29 @@ func (l *Launcher) Run(ctx context.Context, vm *impv1alpha1.ImpVM) Result {
 	return Result{HandoffDone: true, ExitCode: resp.ExitCode}
 }
 
+func (l *Launcher) recordAccepted(ctx context.Context, vm *impv1alpha1.ImpVM) error {
+	base := vm.DeepCopy()
+	vm.Status.RunnerHandoffAccepted = true
+	return l.Client.Status().Patch(ctx, vm, client.MergeFrom(base))
+}
+
+func (l *Launcher) recordFailure(ctx context.Context, vm *impv1alpha1.ImpVM, reason, payload string) {
+	base := vm.DeepCopy()
+	vm.Status.RunnerFailureReason = boundedFailureReason(reason, payload)
+	if err := l.Client.Status().Patch(ctx, vm, client.MergeFrom(base)); err != nil {
+		if l.Log != nil {
+			l.Log.Error("failed to persist runner handoff failure", "err", err)
+		}
+	}
+}
+
+func (l *Launcher) recordOutcome(ctx context.Context, vm *impv1alpha1.ImpVM, exitCode int32, reason string) error {
+	base := vm.DeepCopy()
+	vm.Status.RunnerExitCode = &exitCode
+	vm.Status.RunnerFailureReason = boundedFailureReason(reason, "")
+	return l.Client.Status().Patch(ctx, vm, client.MergeFrom(base))
+}
+
 // dialWithRetry waits for the lazy gRPC client to establish a real HTTP/2
 // connection. Creating a grpc.ClientConn alone does not call the VSOCK dialer.
 func (l *Launcher) dialWithRetry(ctx context.Context) (*grpc.ClientConn, error) {

internal/agent/runnerlaunch/launcher_test.go

@@ -9,6 +9,13 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	impv1alpha1 "github.com/syscode-labs/imp/api/v1alpha1"
 )
 
 func TestDialWithRetryWaitsForRealConnection(t *testing.T) {
@@ -63,3 +70,56 @@ func TestSanitizeRunnerStderrSuccessDoesNotLog(t *testing.T) {
 		t.Fatalf("empty failure stderr = %q, want empty", got)
 	}
 }
+
+func TestRunnerOutcomeIsNonSecretAndRestartSafe(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := impv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	vm := &impv1alpha1.ImpVM{
+		ObjectMeta: metav1.ObjectMeta{Name: "vm", Namespace: "ns", UID: "vm-uid"},
+	}
+	c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm).WithStatusSubresource(vm).Build()
+	launcher := &Launcher{Client: c}
+	if err := launcher.recordAccepted(context.Background(), vm); err != nil {
+		t.Fatalf("recordAccepted: %v", err)
+	}
+	if !vm.Status.RunnerHandoffAccepted {
+		t.Fatal("handoff acceptance was not recorded")
+	}
+	launcher.recordFailure(context.Background(), vm, "runner failed token=secret-value", "secret-value")
+	if strings.Contains(vm.Status.RunnerFailureReason, "secret-value") || strings.Contains(vm.Status.RunnerFailureReason, "token=") {
+		t.Fatalf("failure reason leaked payload: %q", vm.Status.RunnerFailureReason)
+	}
+	code := int32(17)
+	if err := launcher.recordOutcome(context.Background(), vm, code, "runner exited"); err != nil {
+		t.Fatalf("recordOutcome: %v", err)
+	}
+	stored := &impv1alpha1.ImpVM{}
+	if err := c.Get(context.Background(), client.ObjectKeyFromObject(vm), stored); err != nil {
+		t.Fatalf("get stored VM: %v", err)
+	}
+	if !stored.Status.RunnerHandoffAccepted {
+		t.Fatal("stored VM lost the durable handoff marker")
+	}
+	if stored.Status.RunnerExitCode == nil || *stored.Status.RunnerExitCode != code {
+		t.Fatalf("stored exit code = %v, want %d", stored.Status.RunnerExitCode, code)
+	}
+}
+
+func TestRunnerHandoffMarkerRefusesReplay(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := impv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	vm := &impv1alpha1.ImpVM{
+		ObjectMeta: metav1.ObjectMeta{Name: "vm", Namespace: "ns", UID: "vm-uid"},
+		Spec:       impv1alpha1.ImpVMSpec{RunnerConfigSecret: "jitconfig"},
+		Status:     impv1alpha1.ImpVMStatus{RunnerHandoffAccepted: true},
+	}
+	c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm).WithStatusSubresource(vm).Build()
+	result := (&Launcher{Client: c}).Run(context.Background(), vm)
+	if result.Err == nil || !strings.Contains(result.Err.Error(), "refusing replay") {
+		t.Fatalf("Run() error = %v, want durable replay refusal", result.Err)
+	}
+}