feat: Improve git repository context UX (Issue #376)

Enhances the user experience for adding git repositories as context.

Frontend UX Improvements:

1. Enhanced Add Context Modal - Repository options with base/feature branch configuration, protected branch detection, sync configuration
2. Improved Repository Dialog - Branch configuration with branch fetching, protected branch warnings, sync support
3. Enhanced Repository Display - Visual badges, color-coded branch pills, improved layout

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: sallyom <somalley@redhat.com>

Author: sallyom

components/backend/handlers/helpers.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"math"
+	"strings"
 	"time"
 
 	authv1 "k8s.io/api/authorization/v1"
@@ -74,3 +75,127 @@ func ValidateSecretAccess(ctx context.Context, k8sClient kubernetes.Interface, n
 
 	return nil
 }
+
+// isProtectedBranch checks if a branch name is commonly protected
+func isProtectedBranch(branch string) bool {
+	if branch == "" {
+		return false
+	}
+	protectedNames := []string{
+		"main", "master", "develop", "dev", "development",
+		"production", "prod", "staging", "stage", "qa", "test", "stable",
+	}
+	branchLower := strings.ToLower(strings.TrimSpace(branch))
+	for _, protected := range protectedNames {
+		if branchLower == protected {
+			return true
+		}
+	}
+	return false
+}
+
+// isValidGitBranchName validates a user-supplied branch name against git branch naming rules
+// and shell injection risks. Returns true if the branch name is safe to use.
+// Security: This prevents command injection by rejecting shell metacharacters.
+func isValidGitBranchName(branch string) bool {
+	if branch == "" {
+		return false
+	}
+
+	// Reject if longer than 255 characters (git limit)
+	if len(branch) > 255 {
+		return false
+	}
+
+	// Reject shell metacharacters that could lead to command injection
+	// CRITICAL: These characters could break out of git commands in wrapper.py
+	shellMetachars := []rune{';', '&', '|', '$', '`', '\\', '\n', '\r', '\t', '<', '>', '(', ')', '{', '}', '\'', '"', ' '}
+	for _, char := range shellMetachars {
+		if strings.ContainsRune(branch, char) {
+			return false
+		}
+	}
+
+	// Reject git control characters and patterns
+	gitControlChars := []string{"..", "~", "^", ":", "?", "*", "[", "@{"}
+	for _, pattern := range gitControlChars {
+		if strings.Contains(branch, pattern) {
+			return false
+		}
+	}
+
+	// Cannot start or end with dot or slash
+	if strings.HasPrefix(branch, ".") || strings.HasSuffix(branch, ".") ||
+		strings.HasPrefix(branch, "/") || strings.HasSuffix(branch, "/") {
+		return false
+	}
+
+	// Cannot contain consecutive slashes
+	if strings.Contains(branch, "//") {
+		return false
+	}
+
+	// Must contain only valid characters: alphanumeric, dash, underscore, slash, dot
+	for _, r := range branch {
+		valid := (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') ||
+			(r >= '0' && r <= '9') || r == '-' || r == '_' || r == '/' || r == '.'
+		if !valid {
+			return false
+		}
+	}
+
+	return true
+}
+
+// sanitizeBranchName converts a display name to a valid git branch name
+func sanitizeBranchName(name string) string {
+	// Replace spaces with hyphens
+	name = strings.ReplaceAll(name, " ", "-")
+	// Remove or replace invalid characters for git branch names
+	// Valid: alphanumeric, dash, underscore, slash, dot (but not at start/end)
+	var result strings.Builder
+	for _, r := range name {
+		if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') ||
+			(r >= '0' && r <= '9') || r == '-' || r == '_' || r == '/' {
+			result.WriteRune(r)
+		}
+	}
+	sanitized := result.String()
+	// Trim leading/trailing dashes or slashes
+	sanitized = strings.Trim(sanitized, "-/")
+	return sanitized
+}
+
+// generateWorkingBranch generates a working branch name based on the session and repo context
+// Returns the branch name to use for the session
+func generateWorkingBranch(sessionDisplayName, sessionID, requestedBranch string, allowProtectedWork bool) string {
+	// If user explicitly requested a branch
+	if requestedBranch != "" {
+		// Check if it's protected and user hasn't allowed working on it
+		if isProtectedBranch(requestedBranch) && !allowProtectedWork {
+			// Create a temporary working branch to protect the base branch
+			sessionIDShort := sessionID
+			if len(sessionID) > 8 {
+				sessionIDShort = sessionID[:8]
+			}
+			return fmt.Sprintf("work/%s/%s", requestedBranch, sessionIDShort)
+		}
+		// User requested non-protected branch or explicitly allowed protected work
+		return requestedBranch
+	}
+
+	// No branch requested - generate from session name
+	if sessionDisplayName != "" {
+		sanitized := sanitizeBranchName(sessionDisplayName)
+		if sanitized != "" {
+			return sanitized
+		}
+	}
+
+	// Fallback: use session ID
+	sessionIDShort := sessionID
+	if len(sessionID) > 8 {
+		sessionIDShort = sessionID[:8]
+	}
+	return fmt.Sprintf("session-%s", sessionIDShort)
+}

components/backend/handlers/sessions.go

@@ -599,41 +599,135 @@ func CreateSession(c *gin.Context) {
 		if metadata["annotations"] == nil {
 			metadata["annotations"] = make(map[string]interface{})
 		}
-		annotations := metadata["annotations"].(map[string]interface{})
+		annotations, found, err := unstructured.NestedMap(metadata, "annotations")
+		if err != nil || !found {
+			log.Printf("Failed to get annotations from metadata for parent session %s: %v", req.ParentSessionID, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session metadata"})
+			return
+		}
 		annotations["vteam.ambient-code/parent-session-id"] = req.ParentSessionID
+		// Persist annotations back to metadata
+		if err := unstructured.SetNestedMap(metadata, annotations, "annotations"); err != nil {
+			log.Printf("Failed to set annotations for parent session %s: %v", req.ParentSessionID, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session metadata"})
+			return
+		}
+		// Persist metadata back to session
+		if err := unstructured.SetNestedMap(session, metadata, "metadata"); err != nil {
+			log.Printf("Failed to set metadata for session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session metadata"})
+			return
+		}
 		log.Printf("Creating continuation session from parent %s (operator will handle temp pod cleanup)", req.ParentSessionID)
 		// Note: Operator will delete temp pod when session starts (desired-phase=Running)
 	}
 
 	if len(envVars) > 0 {
-		spec := session["spec"].(map[string]interface{})
+		spec, found, err := unstructured.NestedMap(session, "spec")
+		if err != nil || !found {
+			log.Printf("Failed to get spec from session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
 		spec["environmentVariables"] = envVars
+		// Persist spec back to session
+		if err := unstructured.SetNestedMap(session, spec, "spec"); err != nil {
+			log.Printf("Failed to set spec for session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
 	}
 
 	// Interactive flag
 	if req.Interactive != nil {
-		session["spec"].(map[string]interface{})["interactive"] = *req.Interactive
+		spec, found, err := unstructured.NestedMap(session, "spec")
+		if err != nil || !found {
+			log.Printf("Failed to get spec from session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
+		spec["interactive"] = *req.Interactive
+		// Persist spec back to session
+		if err := unstructured.SetNestedMap(session, spec, "spec"); err != nil {
+			log.Printf("Failed to set spec for session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
 	}
 
 	// AutoPushOnComplete flag
 	if req.AutoPushOnComplete != nil {
-		session["spec"].(map[string]interface{})["autoPushOnComplete"] = *req.AutoPushOnComplete
+		spec, found, err := unstructured.NestedMap(session, "spec")
+		if err != nil || !found {
+			log.Printf("Failed to get spec from session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
+		spec["autoPushOnComplete"] = *req.AutoPushOnComplete
+		// Persist spec back to session
+		if err := unstructured.SetNestedMap(session, spec, "spec"); err != nil {
+			log.Printf("Failed to set spec for session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
 	}
 
 	// Set multi-repo configuration on spec (simplified format)
+	// Generate working branch names upfront based on session context
 	{
-		spec := session["spec"].(map[string]interface{})
+		spec, found, err := unstructured.NestedMap(session, "spec")
+		if err != nil || !found {
+			log.Printf("Failed to get spec from session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
 		if len(req.Repos) > 0 {
 			arr := make([]map[string]interface{}, 0, len(req.Repos))
 			for _, r := range req.Repos {
-				m := map[string]interface{}{"url": r.URL}
-				if r.Branch != nil {
-					m["branch"] = *r.Branch
+				// Determine the working branch to use
+				var requestedBranch string
+				if r.WorkingBranch != nil {
+					requestedBranch = strings.TrimSpace(*r.WorkingBranch)
+				} else if r.Branch != nil {
+					requestedBranch = strings.TrimSpace(*r.Branch)
+				}
+
+				// Validate user-supplied branch names to prevent command injection
+				if requestedBranch != "" && !isValidGitBranchName(requestedBranch) {
+					c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("Invalid branch name format: %s", requestedBranch)})
+					return
+				}
+
+				allowProtected := false
+				if r.AllowProtectedWork != nil {
+					allowProtected = *r.AllowProtectedWork
+				}
+
+				// Generate the actual branch name that will be used
+				workingBranch := generateWorkingBranch(
+					req.DisplayName,
+					name, // session name (unique ID)
+					requestedBranch,
+					allowProtected,
+				)
+
+				// Wrap in 'input' object to match runner expectations
+				m := map[string]interface{}{
+					"input": map[string]interface{}{
+						"url":    r.URL,
+						"branch": workingBranch,
+					},
 				}
 				arr = append(arr, m)
 			}
 			spec["repos"] = arr
 		}
+		// Persist spec back to session
+		if err := unstructured.SetNestedMap(session, spec, "spec"); err != nil {
+			log.Printf("Failed to set spec for session %s: %v", name, err)
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+			return
+		}
 	}
 
 	// Add userContext derived from authenticated caller; ignore client-supplied userId
@@ -661,11 +755,23 @@ func CreateSession(c *gin.Context) {
 			if len(groups) == 0 && req.UserContext != nil {
 				groups = req.UserContext.Groups
 			}
-			session["spec"].(map[string]interface{})["userContext"] = map[string]interface{}{
+			spec, found, err := unstructured.NestedMap(session, "spec")
+			if err != nil || !found {
+				log.Printf("Failed to get spec from session %s: %v", name, err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+				return
+			}
+			spec["userContext"] = map[string]interface{}{
 				"userId":      uid,
 				"displayName": displayName,
 				"groups":      groups,
 			}
+			// Persist spec back to session
+			if err := unstructured.SetNestedMap(session, spec, "spec"); err != nil {
+				log.Printf("Failed to set spec for session %s: %v", name, err)
+				c.JSON(http.StatusInternalServerError, gin.H{"error": "Invalid session spec"})
+				return
+			}
 		}
 	}
 
@@ -1406,19 +1512,21 @@ func AddRepo(c *gin.Context) {
 	}
 
 	var req struct {
-		URL    string `json:"url" binding:"required"`
-		Branch string `json:"branch"`
+		URL                string `json:"url" binding:"required"`
+		Branch             string `json:"branch"`
+		WorkingBranch      string `json:"workingBranch"`
+		AllowProtectedWork bool   `json:"allowProtectedWork"`
+		Sync               *struct {
+			URL    string `json:"url"`
+			Branch string `json:"branch"`
+		} `json:"sync"`
 	}
 
 	if err := c.ShouldBindJSON(&req); err != nil {
 		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
 		return
 	}
 
-	if req.Branch == "" {
-		req.Branch = "main"
-	}
-
 	gvr := GetAgenticSessionV1Alpha1Resource()
 	item, err := k8sDyn.Resource(gvr).Namespace(project).Get(context.TODO(), sessionName, v1.GetOptions{})
 	if err != nil {
@@ -1447,10 +1555,52 @@ func AddRepo(c *gin.Context) {
 		repos = []interface{}{}
 	}
 
+	// Determine the requested branch
+	requestedBranch := req.WorkingBranch
+	if requestedBranch == "" {
+		requestedBranch = req.Branch
+	}
+
+	// Validate user-supplied branch names to prevent command injection
+	if requestedBranch != "" && !isValidGitBranchName(requestedBranch) {
+		c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("Invalid branch name format: %s", requestedBranch)})
+		return
+	}
+
+	// Get session display name for branch generation
+	displayName := ""
+	if dn, ok := spec["displayName"].(string); ok {
+		displayName = dn
+	}
+
+	// Generate the actual working branch name
+	workingBranch := generateWorkingBranch(
+		displayName,
+		sessionName,
+		requestedBranch,
+		req.AllowProtectedWork,
+	)
+
+	// Wrap in 'input' object to match runner expectations
 	newRepo := map[string]interface{}{
-		"url":    req.URL,
-		"branch": req.Branch,
+		"input": map[string]interface{}{
+			"url":    req.URL,
+			"branch": workingBranch,
+		},
+	}
+
+	// Add sync configuration if provided
+	if req.Sync != nil && strings.TrimSpace(req.Sync.URL) != "" {
+		syncBranch := strings.TrimSpace(req.Sync.Branch)
+		if syncBranch == "" {
+			syncBranch = "main"
+		}
+		newRepo["sync"] = map[string]interface{}{
+			"url":    strings.TrimSpace(req.Sync.URL),
+			"branch": syncBranch,
+		}
 	}
+
 	repos = append(repos, newRepo)
 	spec["repos"] = repos