refactor: comprehensive code quality improvements (#69)

apps/backend/internal/agentctl/server/process/git.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 
@@ -19,6 +20,30 @@ import (
 // ErrOperationInProgress is returned when a git operation is already in progress.
 var ErrOperationInProgress = errors.New("git operation already in progress")
 
+// ErrInvalidBranchName is returned when a branch name contains invalid characters.
+var ErrInvalidBranchName = errors.New("invalid branch name")
+
+// validBranchNameRegex matches safe git branch names.
+// Allows alphanumeric, hyphens, underscores, slashes, and dots.
+// Disallows: spaces, shell metacharacters, and control characters.
+var validBranchNameRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._/-]*$`)
+
+// isValidBranchName validates that a branch name is safe to use in git commands.
+func isValidBranchName(branch string) bool {
+	if branch == "" || len(branch) > 255 {
+		return false
+	}
+	// Disallow ".." to prevent path traversal
+	if strings.Contains(branch, "..") {
+		return false
+	}
+	// Disallow ending with ".lock"
+	if strings.HasSuffix(branch, ".lock") {
+		return false
+	}
+	return validBranchNameRegex.MatchString(branch)
+}
+
 // GitOperationResult represents the result of a git operation.
 type GitOperationResult struct {
 	Success       bool     `json:"success"`
@@ -237,6 +262,11 @@ func (g *GitOperator) Push(ctx context.Context, force bool, setUpstream bool) (*
 
 // Rebase performs a git rebase onto the specified base branch.
 func (g *GitOperator) Rebase(ctx context.Context, baseBranch string) (*GitOperationResult, error) {
+	// Validate branch name to prevent command injection
+	if !isValidBranchName(baseBranch) {
+		return nil, ErrInvalidBranchName
+	}
+
 	if !g.tryLock("rebase") {
 		return nil, ErrOperationInProgress
 	}
@@ -279,6 +309,11 @@ func (g *GitOperator) Rebase(ctx context.Context, baseBranch string) (*GitOperat
 
 // Merge performs a git merge of the specified base branch.
 func (g *GitOperator) Merge(ctx context.Context, baseBranch string) (*GitOperationResult, error) {
+	// Validate branch name to prevent command injection
+	if !isValidBranchName(baseBranch) {
+		return nil, ErrInvalidBranchName
+	}
+
 	if !g.tryLock("merge") {
 		return nil, ErrOperationInProgress
 	}

apps/backend/internal/agentctl/server/process/workspace_tracker.go

@@ -641,8 +641,12 @@ func (wt *WorkspaceTracker) addDirectoryRecursive(dir string) error {
 
 // GetFileTree returns the file tree for a given path and depth
 func (wt *WorkspaceTracker) GetFileTree(reqPath string, depth int) (*types.FileTreeNode, error) {
-	// Resolve the full path
-	fullPath := filepath.Join(wt.workDir, reqPath)
+	// Resolve the full path with path traversal protection
+	fullPath := filepath.Join(wt.workDir, filepath.Clean(reqPath))
+	cleanWorkDir := filepath.Clean(wt.workDir)
+	if !strings.HasPrefix(fullPath, cleanWorkDir+string(os.PathSeparator)) && fullPath != cleanWorkDir {
+		return nil, fmt.Errorf("path traversal detected")
+	}
 
 	// Check if path exists
 	info, err := os.Stat(fullPath)
@@ -712,8 +716,12 @@ func (wt *WorkspaceTracker) buildFileTreeNode(fullPath, relPath string, info os.
 
 // GetFileContent returns the content of a file
 func (wt *WorkspaceTracker) GetFileContent(reqPath string) (string, int64, error) {
-	// Resolve the full path
-	fullPath := filepath.Join(wt.workDir, reqPath)
+	// Resolve the full path with path traversal protection
+	fullPath := filepath.Join(wt.workDir, filepath.Clean(reqPath))
+	cleanWorkDir := filepath.Clean(wt.workDir)
+	if !strings.HasPrefix(fullPath, cleanWorkDir+string(os.PathSeparator)) && fullPath != cleanWorkDir {
+		return "", 0, fmt.Errorf("path traversal detected")
+	}
 
 	// Check if file exists and is a regular file
 	info, err := os.Stat(fullPath)