Harden runtime-ownership check against ambiguity

isOwnedByActiveRuntime could only ever deny ownership when it read a
RunConfig whose runtime_name positively named a different runtime. Every
ambiguous outcome must default to "owned" so we never silently suppress
reconciliation for a workload we are responsible for.

Check Exists() before reading so a genuinely-missing RunConfig is handled
distinctly from a present-but-unparseable one, trim whitespace-only
runtime_name to the legacy path, and document why the bias toward
ownership is required. Run the eager policy gate before stamping
RuntimeName so a policy denial never dereferences the runtime.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: JAORMX

cmd/thv/app/run.go

@@ -243,17 +243,17 @@ func runSingleServer(ctx context.Context, runFlags *RunFlags, serverOrImage stri
 		return err
 	}
 
-	// Record which runtime owns this workload so that reconciliation logic
-	// does not corrupt its status file when a different runtime is active.
-	runnerConfig.RuntimeName = rt.Name()
-
 	// Enforce policy in the main process before saving state or spawning a
 	// detached worker, so violations surface synchronously with a non-zero
 	// exit code rather than silently failing in the background log.
 	if err := runner.EagerCheckCreateServer(ctx, runnerConfig); err != nil {
 		return fmt.Errorf("server creation blocked by policy: %w", err)
 	}
 
+	// Record which runtime owns this workload so that reconciliation logic
+	// does not corrupt its status file when a different runtime is active.
+	runnerConfig.RuntimeName = rt.Name()
+
 	// Always save the run config to disk before starting (both foreground and detached modes)
 	// NOTE: Save before secrets processing to avoid storing secrets in the state store
 	if err := runnerConfig.SaveState(ctx); err != nil {

pkg/api/v1/workload_service.go

@@ -100,16 +100,16 @@ func (s *WorkloadService) CreateWorkloadFromRequest(ctx context.Context, req *cr
 		return nil, err
 	}
 
-	// Record which runtime owns this workload for cross-runtime reconciliation.
-	runConfig.RuntimeName = s.containerRuntime.Name()
-
 	// Enforce policy before saving state or starting the workload, so
 	// violations are returned as API errors rather than creating the server
 	// in a broken state.
 	if err := runner.EagerCheckCreateServer(ctx, runConfig); err != nil {
 		return nil, fmt.Errorf("server creation blocked by policy: %w", err)
 	}
 
+	// Record which runtime owns this workload for cross-runtime reconciliation.
+	runConfig.RuntimeName = s.containerRuntime.Name()
+
 	// Save the workload state
 	if err := runConfig.SaveState(ctx); err != nil {
 		slog.Error("failed to save workload config", "error", err)

pkg/workloads/statuses/file_status.go

@@ -76,14 +76,30 @@ type fileStatusManager struct {
 
 // isOwnedByActiveRuntime checks whether the named workload was created by the
 // currently active runtime. It loads the runtime_name field from the persisted
-// RunConfig and compares it against f.runtime.Name(). Legacy workloads that
-// predate the runtime_name field (empty string) are conservatively treated as
-// owned by the active runtime so that existing validation behaviour is preserved.
+// RunConfig and compares it against f.runtime.Name().
+//
+// Ownership can only be denied when we positively identify a *different* owning
+// runtime: the RunConfig must exist, decode cleanly, and carry a non-empty
+// runtime_name that differs from the active runtime. Every other case --
+// missing/unreadable config, decode failure, or a legacy workload that predates
+// the runtime_name field (empty string) -- is conservatively treated as owned by
+// the active runtime, preserving the pre-feature validation behaviour. This bias
+// matters: a false "not ours" would silently suppress reconciliation for a
+// workload we are responsible for, leaving it stuck in a stale status.
 func (f *fileStatusManager) isOwnedByActiveRuntime(ctx context.Context, workloadName string) bool {
+	// A workload with no RunConfig cannot have been tagged with another
+	// runtime's name, so it can only be ours (or a legacy workload). Checking
+	// existence explicitly lets us distinguish "no config" from "config present
+	// but unparseable" and mirrors isRemoteWorkload's handling.
+	exists, err := f.runConfigStore.Exists(ctx, workloadName)
+	if err != nil || !exists {
+		return true
+	}
+
 	reader, err := f.runConfigStore.GetReader(ctx, workloadName)
 	if err != nil {
-		// RunConfig missing or unreadable -- assume ours so we don't silently
-		// skip validation for workloads we should be checking.
+		// RunConfig unreadable -- assume ours so we don't silently skip
+		// validation for workloads we should be checking.
 		return true
 	}
 	defer func() {
@@ -99,10 +115,12 @@ func (f *fileStatusManager) isOwnedByActiveRuntime(ctx context.Context, workload
 		return true // can't determine ownership -- assume ours
 	}
 
-	// Empty means legacy workload; treat as owned by active runtime.
-	if config.RuntimeName == "" {
+	// Empty means legacy workload (predates runtime_name); treat as ours.
+	if strings.TrimSpace(config.RuntimeName) == "" {
 		return true
 	}
+
+	// Only deny ownership when another runtime is positively identified.
 	return config.RuntimeName == f.runtime.Name()
 }
 

pkg/workloads/statuses/file_status_test.go

@@ -2389,6 +2389,108 @@ type nopWriteCloser struct {
 
 func (nopWriteCloser) Close() error { return nil }
 
+// TestIsOwnedByActiveRuntime exercises every branch of the ownership check.
+// Ownership may only be denied when a *different* runtime is positively
+// identified; every ambiguous case must default to "owned" so we never silently
+// suppress reconciliation for a workload we are responsible for.
+func TestIsOwnedByActiveRuntime(t *testing.T) {
+	t.Parallel()
+
+	const workloadName = "test-workload"
+	const activeRuntime = "docker"
+
+	tests := []struct {
+		name string
+		// existsResult/existsErr control the runConfigStore.Exists mock.
+		existsResult bool
+		existsErr    error
+		// when the config exists, getReaderErr or runConfigJSON drive GetReader.
+		getReaderErr  error
+		runConfigJSON string
+		expectOwned   bool
+	}{
+		{
+			name:          "owned by active runtime",
+			existsResult:  true,
+			runConfigJSON: `{"runtime_name": "docker"}`,
+			expectOwned:   true,
+		},
+		{
+			name:          "owned by a different runtime is not ours",
+			existsResult:  true,
+			runConfigJSON: `{"runtime_name": "go-microvm"}`,
+			expectOwned:   false,
+		},
+		{
+			name:          "legacy workload with empty runtime_name is assumed ours",
+			existsResult:  true,
+			runConfigJSON: `{"name": "test-workload"}`,
+			expectOwned:   true,
+		},
+		{
+			name:          "whitespace-only runtime_name is treated as legacy",
+			existsResult:  true,
+			runConfigJSON: `{"runtime_name": "   "}`,
+			expectOwned:   true,
+		},
+		{
+			name:         "missing run config is assumed ours",
+			existsResult: false,
+			expectOwned:  true,
+		},
+		{
+			name:        "exists error is assumed ours",
+			existsErr:   errors.New("store unavailable"),
+			expectOwned: true,
+		},
+		{
+			name:         "unreadable run config is assumed ours",
+			existsResult: true,
+			getReaderErr: errors.New("read error"),
+			expectOwned:  true,
+		},
+		{
+			name:          "undecodable run config is assumed ours",
+			existsResult:  true,
+			runConfigJSON: `{bad json`,
+			expectOwned:   true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			manager, mockRuntime, mockRunConfigStore := newTestFileStatusManager(t, ctrl)
+			ctx := t.Context()
+
+			// Name() is only consulted when a non-empty runtime_name is compared.
+			mockRuntime.EXPECT().Name().Return(activeRuntime).AnyTimes()
+
+			mockRunConfigStore.EXPECT().Exists(gomock.Any(), workloadName).
+				Return(tt.existsResult, tt.existsErr)
+
+			// GetReader is only reached when the config exists and Exists succeeds.
+			if tt.existsErr == nil && tt.existsResult {
+				if tt.getReaderErr != nil {
+					mockRunConfigStore.EXPECT().GetReader(gomock.Any(), workloadName).
+						Return(nil, tt.getReaderErr)
+				} else {
+					mockRunConfigStore.EXPECT().GetReader(gomock.Any(), workloadName).DoAndReturn(
+						func(context.Context, string) (io.ReadCloser, error) {
+							return io.NopCloser(strings.NewReader(tt.runConfigJSON)), nil
+						})
+				}
+			}
+
+			assert.Equal(t, tt.expectOwned, manager.isOwnedByActiveRuntime(ctx, workloadName))
+		})
+	}
+}
+
 func TestMigrateRuntimeName(t *testing.T) {
 	t.Parallel()