Skip to content

Commit 5c7d9b7

Browse files
committed
PR Review suggestions
Signed-off-by: sallyom <somalley@redhat.com>
1 parent 9b7b5a7 commit 5c7d9b7

File tree

3 files changed

+41
-2
lines changed

3 files changed

+41
-2
lines changed

components/backend/crd/bugfix.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package crd
33
import (
44
"context"
55
"fmt"
6+
"log"
67

78
"ambient-code-backend/types"
89

@@ -113,8 +114,16 @@ func GetProjectBugFixWorkflowCR(dyn dynamic.Interface, project, id string) (*typ
113114

114115
spec, found, _ := unstructured.NestedMap(obj.Object, "spec")
115116
if found {
117+
// Parse githubIssueNumber with type safety - handle both int64 and float64
118+
// JSON unmarshaling can produce either depending on the source
116119
if val, ok := spec["githubIssueNumber"].(int64); ok {
117120
workflow.GithubIssueNumber = int(val)
121+
} else if val, ok := spec["githubIssueNumber"].(float64); ok {
122+
workflow.GithubIssueNumber = int(val)
123+
} else if spec["githubIssueNumber"] != nil {
124+
// Log warning if type assertion fails - helps debug unexpected types
125+
log.Printf("Warning: githubIssueNumber has unexpected type %T in BugFixWorkflow %s/%s",
126+
spec["githubIssueNumber"], project, id)
118127
}
119128
if val, ok := spec["githubIssueURL"].(string); ok {
120129
workflow.GithubIssueURL = val

components/backend/handlers/bugfix/create.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"log"
66
"net/http"
7+
"regexp"
78
"strings"
89
"time"
910

@@ -24,6 +25,31 @@ var (
2425
GetProjectSettingsResource func() schema.GroupVersionResource
2526
)
2627

28+
// validBranchNameRegex defines allowed characters in branch names
29+
// Allows: letters, numbers, hyphens, underscores, forward slashes, and dots
30+
// Prevents: shell metacharacters, backticks, quotes, semicolons, pipes, etc.
31+
var validBranchNameRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.-]+$`)
32+
33+
// validateBranchName checks if a branch name is safe to use in git operations
34+
// Returns error if the branch name contains potentially dangerous characters
35+
func validateBranchName(branchName string) error {
36+
if branchName == "" {
37+
return fmt.Errorf("branch name cannot be empty")
38+
}
39+
if !validBranchNameRegex.MatchString(branchName) {
40+
return fmt.Errorf("branch name contains invalid characters (allowed: a-z, A-Z, 0-9, /, _, -, .)")
41+
}
42+
// Prevent branch names that start with special characters
43+
if strings.HasPrefix(branchName, ".") || strings.HasPrefix(branchName, "-") {
44+
return fmt.Errorf("branch name cannot start with '.' or '-'")
45+
}
46+
// Prevent branch names with ".." (path traversal) or "//" (double slashes)
47+
if strings.Contains(branchName, "..") || strings.Contains(branchName, "//") {
48+
return fmt.Errorf("branch name cannot contain '..' or '//'")
49+
}
50+
return nil
51+
}
52+
2753
// CreateProjectBugFixWorkflow handles POST /api/projects/:projectName/bugfix-workflows
2854
// Creates a new BugFix Workspace from either GitHub Issue URL or text description
2955
func CreateProjectBugFixWorkflow(c *gin.Context) {
@@ -165,6 +191,11 @@ func CreateProjectBugFixWorkflow(c *gin.Context) {
165191
branch := "main"
166192
if req.BranchName != nil && *req.BranchName != "" {
167193
branch = *req.BranchName
194+
// Validate user-provided branch name for security
195+
if err := validateBranchName(branch); err != nil {
196+
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid branch name", "details": err.Error()})
197+
return
198+
}
168199
} else {
169200
// Auto-generate branch name: bugfix/gh-{issue-number}
170201
branch = fmt.Sprintf("bugfix/gh-%d", githubIssue.Number)

components/backend/handlers/bugfix/sessions.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package bugfix
22

33
import (
4-
"context"
54
"fmt"
65
"log"
76
"net/http"
@@ -427,7 +426,7 @@ func CreateProjectBugFixWorkflowSession(c *gin.Context) {
427426
},
428427
}
429428

430-
created, err := reqDyn.Resource(gvr).Namespace(project).Create(context.TODO(), sessionObj, v1.CreateOptions{})
429+
created, err := reqDyn.Resource(gvr).Namespace(project).Create(c.Request.Context(), sessionObj, v1.CreateOptions{})
431430
if err != nil {
432431
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create session", "details": err.Error()})
433432
return

0 commit comments

Comments
 (0)