Handle non-root Kubernetes exec user context

pkg/runtime/k8s_parity_test.go

@@ -17,14 +17,14 @@ package runtime
 import (
 	"context"
 	"embed"
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/scion/pkg/api"
 	"github.com/GoogleCloudPlatform/scion/pkg/k8s"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sruntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/dynamic/fake"
@@ -373,48 +373,123 @@ func TestCreateAuthFileSecret(t *testing.T) {
 
 // --- K8s exec user context parity (matches Docker/Podman --user scion) ---
 
-func TestK8sExec_WrapsCommandWithSu(t *testing.T) {
-	// Verify that Exec wraps commands with su to run as the scion user,
-	// matching the --user scion flag used by Docker/Podman runtimes.
+func TestK8sExec_BuildCommandForMatchingUserReturnsRawArgv(t *testing.T) {
 	cmd := []string{"tmux", "send-keys", "-t", "scion:0", "hello world", "Enter"}
 
-	// Simulate the wrapping logic from Exec
-	quoted := make([]string, len(cmd))
-	for i, arg := range cmd {
-		quoted[i] = fmt.Sprintf("'%s'", strings.ReplaceAll(arg, "'", "'\"'\"'"))
+	got := buildExecCommandForUser("scion", "scion", cmd)
+	if len(got) != len(cmd) {
+		t.Fatalf("expected raw argv %v, got %v", cmd, got)
 	}
-	suCmd := []string{"su", "-", "scion", "-c", strings.Join(quoted, " ")}
-
-	if suCmd[0] != "su" || suCmd[1] != "-" || suCmd[2] != "scion" || suCmd[3] != "-c" {
-		t.Fatalf("expected su - scion -c prefix, got: %v", suCmd[:4])
+	for i := range cmd {
+		if got[i] != cmd[i] {
+			t.Fatalf("expected raw argv %v, got %v", cmd, got)
+		}
 	}
+}
 
-	// The -c argument should contain all original args, properly quoted
-	shellCmd := suCmd[4]
+func TestK8sExec_BuildCommandForDifferentUserFallsBackToSu(t *testing.T) {
+	cmd := []string{"tmux", "attach", "-t", "scion"}
+
+	got := buildExecCommandForUser("root", "scion", cmd)
+	if len(got) != 5 {
+		t.Fatalf("expected su argv, got %v", got)
+	}
+	if got[0] != "su" || got[1] != "-" || got[2] != "scion" || got[3] != "-c" {
+		t.Fatalf("expected su - scion -c prefix, got %v", got[:4])
+	}
+	shellCmd := got[4]
 	for _, arg := range cmd {
 		if !strings.Contains(shellCmd, arg) {
-			t.Errorf("shell command %q should contain argument %q", shellCmd, arg)
+			t.Errorf("su shell command %q should contain argument %q", shellCmd, arg)
 		}
 	}
 }
 
 func TestK8sExec_QuotesSingleQuotesInArgs(t *testing.T) {
-	cmd := []string{"echo", "it's a test"}
-
-	quoted := make([]string, len(cmd))
-	for i, arg := range cmd {
-		quoted[i] = fmt.Sprintf("'%s'", strings.ReplaceAll(arg, "'", "'\"'\"'"))
-	}
-	shellCmd := strings.Join(quoted, " ")
+	shellCmd := shellJoin([]string{"echo", "it's a test"})
 
 	// The single quote in "it's" should be escaped
 	if !strings.Contains(shellCmd, "'\"'\"'") {
 		t.Errorf("expected escaped single quote in %q", shellCmd)
 	}
 }
 
+func TestK8sExecTargetUsername_UsesAnnotationWhenPresent(t *testing.T) {
+	pod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:   "scion",
+			Name:        "agent",
+			Annotations: map[string]string{"scion.username": "carver"},
+		},
+	}
+	if got := execTargetUsername(pod); got != "carver" {
+		t.Fatalf("got username %q, want carver", got)
+	}
+}
+
+func TestK8sExecTargetUsername_FallsBackWhenAnnotationInvalid(t *testing.T) {
+	pod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:   "scion",
+			Name:        "agent",
+			Annotations: map[string]string{"scion.username": "--help"},
+		},
+	}
+	if got := execTargetUsername(pod); got != "scion" {
+		t.Fatalf("got username %q, want scion", got)
+	}
+}
+
+func TestK8sPodRunsAsNonRoot(t *testing.T) {
+	runAsUser := int64(1000)
+	runAsNonRoot := true
+	tests := []struct {
+		name string
+		pod  *corev1.Pod
+		want bool
+	}{
+		{
+			name: "pod security context runAsUser",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					SecurityContext: &corev1.PodSecurityContext{RunAsUser: &runAsUser},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "container security context runAsNonRoot",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:            "agent",
+						SecurityContext: &corev1.SecurityContext{RunAsNonRoot: &runAsNonRoot},
+					}},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "no security context",
+			pod: &corev1.Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{Name: "agent"}},
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := podRunsAsNonRoot(tt.pod, "agent"); got != tt.want {
+				t.Fatalf("got %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestK8sAttach_ResolvesUsernameFromAnnotations(t *testing.T) {
-	// Verify that Attach reads the username from scion.username annotation
 	tests := []struct {
 		name        string
 		annotations map[string]string
@@ -435,17 +510,18 @@ func TestK8sAttach_ResolvesUsernameFromAnnotations(t *testing.T) {
 			annotations: map[string]string{"scion.username": ""},
 			wantUser:    "scion",
 		},
+		{
+			name:        "defaults to scion when annotation invalid",
+			annotations: map[string]string{"scion.username": "bad user"},
+			wantUser:    "scion",
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Simulate the username resolution logic from Attach
-			username := "scion"
-			if u, ok := tt.annotations["scion.username"]; ok && u != "" {
-				username = u
-			}
-			if username != tt.wantUser {
-				t.Errorf("got username %q, want %q", username, tt.wantUser)
+			pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: tt.annotations}}
+			if got := execTargetUsername(pod); got != tt.wantUser {
+				t.Errorf("got username %q, want %q", got, tt.wantUser)
 			}
 		})
 	}

pkg/runtime/k8s_runtime.go

@@ -26,6 +26,7 @@ import (
 	"os/exec"
 	"os/signal"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"syscall"
 	"time"
@@ -73,6 +74,96 @@ func (r *KubernetesRuntime) ExecUser() string {
 	return "scion"
 }
 
+var validExecUsername = regexp.MustCompile(`^[a-z_][a-z0-9_-]{0,31}$`)
+
+func execTargetUsername(pod *corev1.Pod) string {
+	if pod != nil {
+		if u := strings.TrimSpace(pod.Annotations["scion.username"]); u != "" && validExecUsername.MatchString(u) {
+			return u
+		}
+	}
+	return "scion"
+}
+
+func shellQuote(arg string) string {
+	return fmt.Sprintf("'%s'", strings.ReplaceAll(arg, "'", "'\"'\"'"))
+}
+
+func shellJoin(args []string) string {
+	quoted := make([]string, len(args))
+	for i, arg := range args {
+		quoted[i] = shellQuote(arg)
+	}
+	return strings.Join(quoted, " ")
+}
+
+func buildExecCommandForUser(currentUser, targetUser string, cmd []string) []string {
+	if targetUser == "" {
+		targetUser = "scion"
+	}
+	if currentUser == "" || currentUser == targetUser || targetUser == "root" {
+		return append([]string(nil), cmd...)
+	}
+	return []string{"su", "-", targetUser, "-c", shellJoin(cmd)}
+}
+
+func podRunsAsNonRoot(pod *corev1.Pod, containerName string) bool {
+	if pod == nil {
+		return false
+	}
+	if pod.Spec.SecurityContext != nil {
+		if pod.Spec.SecurityContext.RunAsUser != nil {
+			return *pod.Spec.SecurityContext.RunAsUser != 0
+		}
+		if pod.Spec.SecurityContext.RunAsNonRoot != nil && *pod.Spec.SecurityContext.RunAsNonRoot {
+			return true
+		}
+	}
+	for _, container := range pod.Spec.Containers {
+		if container.Name != containerName {
+			continue
+		}
+		if container.SecurityContext == nil {
+			return false
+		}
+		if container.SecurityContext.RunAsUser != nil {
+			return *container.SecurityContext.RunAsUser != 0
+		}
+		if container.SecurityContext.RunAsNonRoot != nil && *container.SecurityContext.RunAsNonRoot {
+			return true
+		}
+		return false
+	}
+	return false
+}
+
+func (r *KubernetesRuntime) currentExecUser(ctx context.Context, namespace, podName string) (string, error) {
+	out, err := r.execInPod(ctx, namespace, podName, []string{"id", "-un"})
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSpace(out), nil
+}
+
+func (r *KubernetesRuntime) commandForExec(ctx context.Context, namespace, podName string, cmd []string) ([]string, error) {
+	pod, err := r.Client.Clientset.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	targetUser := execTargetUsername(pod)
+	// Probe the live pod user at exec time instead of caching it because the
+	// effective exec user is a property of the running container state.
+	currentUser, err := r.currentExecUser(ctx, namespace, podName)
+	if err == nil && currentUser != "" {
+		return buildExecCommandForUser(currentUser, targetUser, cmd), nil
+	}
+	if podRunsAsNonRoot(pod, "agent") {
+		return append([]string(nil), cmd...), nil
+	}
+	return buildExecCommandForUser("root", targetUser, cmd), nil
+}
+
 // resolveNamespace determines the namespace for a pod by looking up the
 // scion.namespace annotation on the pod itself. Falls back to DefaultNamespace
 // if the pod is not found or has no annotation.
@@ -302,8 +393,7 @@ func (r *KubernetesRuntime) Run(ctx context.Context, config RunConfig) (string,
 		// Fix ownership: tar extraction runs as root via K8s exec, so synced
 		// files are owned by root. chown them to the scion user so the
 		// privilege-dropped harness process can access its home directory.
-		chownCmd := fmt.Sprintf("chown -R %s:%s %s", config.UnixUsername, config.UnixUsername, destHome)
-		if _, err := r.execInPod(ctx, namespace, createdPod.Name, []string{"sh", "-c", chownCmd}); err != nil {
+		if _, err := r.execInPod(ctx, namespace, createdPod.Name, []string{"chown", "-R", fmt.Sprintf("%s:%s", config.UnixUsername, config.UnixUsername), destHome}); err != nil {
 			runtimeLog.Debug("Failed to chown home directory (non-fatal)", "error", err)
 		}
 	}
@@ -318,8 +408,7 @@ func (r *KubernetesRuntime) Run(ctx context.Context, config RunConfig) (string,
 			return createdPod.Name, fmt.Errorf("failed to sync workspace: %w", err)
 		}
 		// Fix workspace ownership for the scion user
-		chownCmd := fmt.Sprintf("chown -R %s:%s /workspace", config.UnixUsername, config.UnixUsername)
-		if _, err := r.execInPod(ctx, namespace, createdPod.Name, []string{"sh", "-c", chownCmd}); err != nil {
+		if _, err := r.execInPod(ctx, namespace, createdPod.Name, []string{"chown", "-R", fmt.Sprintf("%s:%s", config.UnixUsername, config.UnixUsername), "/workspace"}); err != nil {
 			runtimeLog.Debug("Failed to chown workspace (non-fatal)", "error", err)
 		}
 	}
@@ -1719,16 +1808,14 @@ func (r *KubernetesRuntime) Attach(ctx context.Context, id string) error {
 		Namespace(namespace).
 		SubResource("exec")
 
-	// Determine the container username so we attach as the correct user
-	// (K8s exec has no --user flag; we use su to switch from root).
-	username := "scion"
-	if u, ok := agent.Annotations["scion.username"]; ok && u != "" {
-		username = u
+	execCmd, err := r.commandForExec(ctx, namespace, podName, []string{"tmux", "attach", "-t", "scion"})
+	if err != nil {
+		return err
 	}
 
 	option := &corev1.PodExecOptions{
 		Container: "agent",
-		Command:   []string{"su", "-", username, "-c", "tmux attach -t scion"},
+		Command:   execCmd,
 		Stdin:     true,
 		Stdout:    true,
 		Stderr:    true,
@@ -1982,17 +2069,14 @@ func (r *KubernetesRuntime) Exec(ctx context.Context, id string, cmd []string) (
 		Namespace(namespace).
 		SubResource("exec")
 
-	// Wrap command with su to run as the scion user (K8s exec has no --user flag).
-	// Shell-quote each argument to handle spaces and special characters.
-	quoted := make([]string, len(cmd))
-	for i, arg := range cmd {
-		quoted[i] = fmt.Sprintf("'%s'", strings.ReplaceAll(arg, "'", "'\"'\"'"))
+	execCmd, err := r.commandForExec(ctx, namespace, podName, cmd)
+	if err != nil {
+		return "", err
 	}
-	suCmd := []string{"su", "-", "scion", "-c", strings.Join(quoted, " ")}
 
 	option := &corev1.PodExecOptions{
 		Container: "agent",
-		Command:   suCmd,
+		Command:   execCmd,
 		Stdin:     false,
 		Stdout:    true,
 		Stderr:    true,