diff --git a/components/backend/handlers/helpers.go b/components/backend/handlers/helpers.go index c251e2504..cb452dfbb 100644 --- a/components/backend/handlers/helpers.go +++ b/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) +} diff --git a/components/backend/handlers/sessions.go b/components/backend/handlers/sessions.go index 111d97bb3..5f79c874e 100644 --- a/components/backend/handlers/sessions.go +++ b/components/backend/handlers/sessions.go @@ -599,6 +599,7 @@ func CreateSession(c *gin.Context) { if metadata["annotations"] == nil { metadata["annotations"] = make(map[string]interface{}) } + // Direct map access for plain maps (no need for unstructured helpers) annotations := metadata["annotations"].(map[string]interface{}) annotations["vteam.ambient-code/parent-session-id"] = req.ParentSessionID log.Printf("Creating continuation session from parent %s (operator will handle temp pod cleanup)", req.ParentSessionID) @@ -606,34 +607,69 @@ func CreateSession(c *gin.Context) { } if len(envVars) > 0 { + // Direct map access for plain maps (no need for unstructured helpers) spec := session["spec"].(map[string]interface{}) spec["environmentVariables"] = envVars } // Interactive flag if req.Interactive != nil { - session["spec"].(map[string]interface{})["interactive"] = *req.Interactive + // Direct map access for plain maps (no need for unstructured helpers) + spec := session["spec"].(map[string]interface{}) + spec["interactive"] = *req.Interactive } // AutoPushOnComplete flag if req.AutoPushOnComplete != nil { - session["spec"].(map[string]interface{})["autoPushOnComplete"] = *req.AutoPushOnComplete + // Direct map access for plain maps (no need for unstructured helpers) + spec := session["spec"].(map[string]interface{}) + spec["autoPushOnComplete"] = *req.AutoPushOnComplete } // Set multi-repo configuration on spec (simplified format) - { + // Generate working branch names upfront based on session context + if len(req.Repos) > 0 { + // Direct map access for plain maps (no need for unstructured helpers) spec := session["spec"].(map[string]interface{}) - 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 - } - arr = append(arr, m) + arr := make([]map[string]interface{}, 0, len(req.Repos)) + for _, r := range req.Repos { + // 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 } - spec["repos"] = arr + + 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 } // Add userContext derived from authenticated caller; ignore client-supplied userId @@ -661,7 +697,9 @@ 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{}{ + // Direct map access for plain maps (no need for unstructured helpers) + spec := session["spec"].(map[string]interface{}) + spec["userContext"] = map[string]interface{}{ "userId": uid, "displayName": displayName, "groups": groups, @@ -1406,8 +1444,14 @@ 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 { @@ -1415,10 +1459,6 @@ func AddRepo(c *gin.Context) { 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 +1487,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 diff --git a/components/backend/types/session.go b/components/backend/types/session.go index 1ee23676b..9aa152814 100644 --- a/components/backend/types/session.go +++ b/components/backend/types/session.go @@ -28,8 +28,10 @@ type AgenticSessionSpec struct { // SimpleRepo represents a simplified repository configuration type SimpleRepo struct { - URL string `json:"url"` - Branch *string `json:"branch,omitempty"` + URL string `json:"url"` + Branch *string `json:"branch,omitempty"` + WorkingBranch *string `json:"workingBranch,omitempty"` // User-requested working branch (input only) + AllowProtectedWork *bool `json:"allowProtectedWork,omitempty"` // Allow work directly on protected branches (input only) } type AgenticSessionStatus struct { diff --git a/components/frontend/package-lock.json b/components/frontend/package-lock.json index 82f2f638f..72b7d22a1 100644 --- a/components/frontend/package-lock.json +++ b/components/frontend/package-lock.json @@ -12,6 +12,7 @@ "@radix-ui/react-accordion": "^1.2.12", "@radix-ui/react-avatar": "^1.1.10", "@radix-ui/react-checkbox": "^1.3.3", + "@radix-ui/react-collapsible": "^1.1.12", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-label": "^2.1.7", "@radix-ui/react-progress": "^1.1.7", diff --git a/components/frontend/package.json b/components/frontend/package.json index 9deafaa57..384cd3f00 100644 --- a/components/frontend/package.json +++ b/components/frontend/package.json @@ -13,6 +13,7 @@ "@radix-ui/react-accordion": "^1.2.12", "@radix-ui/react-avatar": "^1.1.10", "@radix-ui/react-checkbox": "^1.3.3", + "@radix-ui/react-collapsible": "^1.1.12", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-label": "^2.1.7", "@radix-ui/react-progress": "^1.1.7", diff --git a/components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/accordions/repositories-accordion.tsx b/components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/accordions/repositories-accordion.tsx index 5724a08a7..5d12b04bf 100644 --- a/components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/accordions/repositories-accordion.tsx +++ b/components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/accordions/repositories-accordion.tsx @@ -1,14 +1,19 @@ "use client"; import { useState } from "react"; -import { GitBranch, X, Link, Loader2, CloudUpload } from "lucide-react"; +import { GitBranch, X, Link, Loader2, CloudUpload, GitMerge, Shield } from "lucide-react"; import { AccordionItem, AccordionTrigger, AccordionContent } from "@/components/ui/accordion"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; type Repository = { url: string; - branch?: string; + branch?: string; // The actual working branch (generated by backend) + allowProtectedWork?: boolean; + sync?: { + url: string; + branch?: string; + }; }; type UploadedFile = { @@ -78,7 +83,7 @@ export function RepositoriesAccordion({
Add additional context to improve AI responses.
- + {/* Context Items List (Repos + Uploaded Files) */} {totalContextItems === 0 ? (