Standardize path validation across file operations to prevent path traversal

CONTRIBUTING.md

@@ -169,6 +169,36 @@ The agent places validation logic appropriately:
 
 See [Validation Architecture](scratchpad/validation-architecture.md) for the complete decision tree.
 
+#### File Path Security
+
+All file operations must validate paths to prevent path traversal attacks:
+
+**Use `fileutil.ValidateAbsolutePath` before file operations:**
+
+```go
+import "github.com/github/gh-aw/pkg/fileutil"
+
+// Validate path before reading/writing files
+cleanPath, err := fileutil.ValidateAbsolutePath(userInputPath)
+if err != nil {
+    return fmt.Errorf("invalid path: %w", err)
+}
+content, err := os.ReadFile(cleanPath)
+```
+
+**Security checks performed:**
+- Normalizes path using `filepath.Clean` (removes `.` and `..` components)
+- Verifies path is absolute (blocks relative path traversal)
+- Returns descriptive errors for invalid paths
+
+**When to use:**
+- Before `os.ReadFile`, `os.WriteFile`, `os.Stat`, `os.Open`
+- Before `os.MkdirAll` or other directory operations
+- After constructing paths with `filepath.Join`
+- When processing user-provided file paths
+
+This provides defense-in-depth against path traversal vulnerabilities (e.g., `../../../etc/passwd`).
+
 #### CLI Breaking Changes
 
 The agent evaluates whether changes are breaking:

pkg/cli/commands.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/workflow"
 )
@@ -222,6 +223,13 @@ func NewWorkflow(workflowName string, verbose bool, force bool) error {
 	githubWorkflowsDir := filepath.Join(workingDir, constants.GetWorkflowDir())
 	commandsLog.Printf("Creating workflows directory: %s", githubWorkflowsDir)
 
+	// Validate the directory path
+	githubWorkflowsDir, err = fileutil.ValidateAbsolutePath(githubWorkflowsDir)
+	if err != nil {
+		commandsLog.Printf("Invalid workflows directory path: %v", err)
+		return fmt.Errorf("invalid workflows directory path: %w", err)
+	}
+
 	if err := os.MkdirAll(githubWorkflowsDir, 0755); err != nil {
 		commandsLog.Printf("Failed to create workflows directory: %v", err)
 		return fmt.Errorf("failed to create .github/workflows directory: %w", err)
@@ -231,6 +239,13 @@ func NewWorkflow(workflowName string, verbose bool, force bool) error {
 	destFile := filepath.Join(githubWorkflowsDir, workflowName+".md")
 	commandsLog.Printf("Destination file: %s", destFile)
 
+	// Validate the destination file path
+	destFile, err = fileutil.ValidateAbsolutePath(destFile)
+	if err != nil {
+		commandsLog.Printf("Invalid destination file path: %v", err)
+		return fmt.Errorf("invalid destination file path: %w", err)
+	}
+
 	// Check if destination file already exists
 	if _, err := os.Stat(destFile); err == nil && !force {
 		commandsLog.Printf("Workflow file already exists and force=false: %s", destFile)

pkg/cli/completions.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 	"github.com/github/gh-aw/pkg/workflow"
@@ -16,12 +17,10 @@ var completionsLog = logger.New("cli:completions")
 // getWorkflowDescription extracts the description field from a workflow's frontmatter
 // Returns empty string if the description is not found or if there's an error reading the file
 func getWorkflowDescription(filePath string) string {
-	// Sanitize the filepath to prevent path traversal attacks
-	cleanPath := filepath.Clean(filePath)
-
-	// Verify the path is absolute to prevent relative path traversal
-	if !filepath.IsAbs(cleanPath) {
-		completionsLog.Printf("Invalid workflow file path (not absolute): %s", filePath)
+	// Validate the path for security
+	cleanPath, err := fileutil.ValidateAbsolutePath(filePath)
+	if err != nil {
+		completionsLog.Printf("Invalid workflow file path: %v", err)
 		return ""
 	}
 

pkg/cli/git.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/charmbracelet/huh"
 	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/logger"
 )
 
@@ -43,6 +44,12 @@ func findGitRootForPath(path string) (string, error) {
 		return "", fmt.Errorf("failed to get absolute path: %w", err)
 	}
 
+	// Validate the absolute path
+	absPath, err = fileutil.ValidateAbsolutePath(absPath)
+	if err != nil {
+		return "", fmt.Errorf("invalid path: %w", err)
+	}
+
 	// Use the directory containing the file
 	dir := filepath.Dir(absPath)
 
@@ -117,6 +124,13 @@ func getRepositorySlugFromRemoteForPath(path string) string {
 		return ""
 	}
 
+	// Validate the absolute path
+	absPath, err = fileutil.ValidateAbsolutePath(absPath)
+	if err != nil {
+		gitLog.Printf("Invalid path: %v", err)
+		return ""
+	}
+
 	// Use the directory containing the file
 	dir := filepath.Dir(absPath)
 
@@ -379,8 +393,12 @@ func ToGitRootRelativePath(path string) string {
 		gitLog.Printf("Converted to absolute path: %s", absPath)
 	}
 
-	// Clean the absolute path
-	absPath = filepath.Clean(absPath)
+	// Validate and clean the absolute path
+	absPath, err := fileutil.ValidateAbsolutePath(absPath)
+	if err != nil {
+		gitLog.Printf("Invalid path: %v", err)
+		return path
+	}
 
 	// Find the git root by looking for .github directory
 	// Walk up the directory tree to find it

pkg/cli/run_push.go

@@ -12,6 +12,7 @@ import (
 	"github.com/github/gh-aw/pkg/stringutil"
 
 	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 )
@@ -36,6 +37,13 @@ func collectWorkflowFiles(ctx context.Context, workflowPath string, verbose bool
 	}
 	runPushLog.Printf("Resolved absolute workflow path: %s", absWorkflowPath)
 
+	// Validate the absolute path
+	absWorkflowPath, err = fileutil.ValidateAbsolutePath(absWorkflowPath)
+	if err != nil {
+		runPushLog.Printf("Invalid workflow path: %v", err)
+		return nil, fmt.Errorf("invalid workflow path: %w", err)
+	}
+
 	// Add the workflow .md file
 	files[absWorkflowPath] = true
 	runPushLog.Printf("Added workflow file: %s", absWorkflowPath)
@@ -173,6 +181,13 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) {
 	}
 	runPushLog.Printf("Resolved absolute path: %s", absWorkflowPath)
 
+	// Validate the absolute path
+	absWorkflowPath, err = fileutil.ValidateAbsolutePath(absWorkflowPath)
+	if err != nil {
+		runPushLog.Printf("Invalid workflow path: %v", err)
+		return nil, fmt.Errorf("invalid workflow path: %w", err)
+	}
+
 	lockFilePath := stringutil.MarkdownToLockFile(absWorkflowPath)
 	runPushLog.Printf("Expected lock file path: %s", lockFilePath)
 	status := &LockFileStatus{
@@ -481,8 +496,14 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string,
 		// Normalize the path
 		absPath, err := filepath.Abs(file)
 		if err == nil {
-			ourFiles[absPath] = true
-			runPushLog.Printf("Added to our files map: %s (absolute: %s)", file, absPath)
+			// Validate the absolute path
+			validPath, validErr := fileutil.ValidateAbsolutePath(absPath)
+			if validErr == nil {
+				ourFiles[validPath] = true
+				runPushLog.Printf("Added to our files map: %s (absolute: %s)", file, validPath)
+			} else {
+				runPushLog.Printf("Failed to validate path for %s: %v", absPath, validErr)
+			}
 		} else {
 			runPushLog.Printf("Failed to get absolute path for %s: %v", file, err)
 		}
@@ -497,9 +518,13 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string,
 		runPushLog.Printf("Checking staged file: %s", stagedFile)
 		// Try both absolute and relative paths
 		absStagedPath, err := filepath.Abs(stagedFile)
-		if err == nil && ourFiles[absStagedPath] {
-			runPushLog.Printf("Staged file %s matches our file %s (absolute)", stagedFile, absStagedPath)
-			continue
+		if err == nil {
+			// Validate the staged path
+			validPath, validErr := fileutil.ValidateAbsolutePath(absStagedPath)
+			if validErr == nil && ourFiles[validPath] {
+				runPushLog.Printf("Staged file %s matches our file %s (absolute)", stagedFile, validPath)
+				continue
+			}
 		}
 		if ourFiles[stagedFile] {
 			runPushLog.Printf("Staged file %s matches our file (relative)", stagedFile)

pkg/cli/trial_repository.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/workflow"
 )
@@ -174,6 +175,12 @@ func cloneTrialHostRepository(repoSlug string, verbose bool) (string, error) {
 	// Create temporary directory
 	tempDir := filepath.Join(os.TempDir(), fmt.Sprintf("gh-aw-trial-%x", time.Now().UnixNano()))
 
+	// Validate the temporary directory path
+	tempDir, err := fileutil.ValidateAbsolutePath(tempDir)
+	if err != nil {
+		return "", fmt.Errorf("invalid temporary directory path: %w", err)
+	}
+
 	if verbose {
 		fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Cloning host repository to: %s", tempDir)))
 	}
@@ -287,20 +294,39 @@ func installLocalWorkflowInTrialMode(originalDir, tempDir string, parsedSpec *Wo
 	// Construct the source path (relative to original directory)
 	sourcePath := filepath.Join(originalDir, parsedSpec.WorkflowPath)
 
+	// Validate source path
+	sourcePath, err := fileutil.ValidateAbsolutePath(sourcePath)
+	if err != nil {
+		return fmt.Errorf("invalid source path: %w", err)
+	}
+
 	// Check if the source file exists
 	if _, err := os.Stat(sourcePath); os.IsNotExist(err) {
 		return fmt.Errorf("local workflow file does not exist: %s", sourcePath)
 	}
 
 	// Create the workflows directory in the temp directory
 	workflowsDir := filepath.Join(tempDir, constants.GetWorkflowDir())
+
+	// Validate workflows directory path
+	workflowsDir, err = fileutil.ValidateAbsolutePath(workflowsDir)
+	if err != nil {
+		return fmt.Errorf("invalid workflows directory path: %w", err)
+	}
+
 	if err := os.MkdirAll(workflowsDir, 0755); err != nil {
 		return fmt.Errorf("failed to create workflows directory: %w", err)
 	}
 
 	// Construct the destination path
 	destPath := filepath.Join(workflowsDir, parsedSpec.WorkflowName+".md")
 
+	// Validate destination path
+	destPath, err = fileutil.ValidateAbsolutePath(destPath)
+	if err != nil {
+		return fmt.Errorf("invalid destination path: %w", err)
+	}
+
 	// Read the source file
 	content, err := os.ReadFile(sourcePath)
 	if err != nil {
@@ -339,6 +365,12 @@ func modifyWorkflowForTrialMode(tempDir, workflowName, logicalRepoSlug string, v
 	// Find the workflow markdown file
 	workflowPath := filepath.Join(tempDir, constants.GetWorkflowDir(), fmt.Sprintf("%s.md", workflowName))
 
+	// Validate workflow path
+	workflowPath, err := fileutil.ValidateAbsolutePath(workflowPath)
+	if err != nil {
+		return fmt.Errorf("invalid workflow path: %w", err)
+	}
+
 	content, err := os.ReadFile(workflowPath)
 	if err != nil {
 		return fmt.Errorf("failed to read workflow file: %w", err)

pkg/fileutil/fileutil.go

@@ -0,0 +1,45 @@
+// Package fileutil provides utility functions for working with file paths and file operations.
+package fileutil
+
+import (
+	"fmt"
+	"path/filepath"
+)
+
+// ValidateAbsolutePath validates that a file path is absolute and safe to use.
+// It performs the following security checks:
+//   - Cleans the path using filepath.Clean to normalize . and .. components
+//   - Verifies the path is absolute to prevent relative path traversal attacks
+//
+// Returns the cleaned absolute path if validation succeeds, or an error if:
+//   - The path is empty
+//   - The path is relative (not absolute)
+//
+// This function should be used before any file operations (read, write, stat, etc.)
+// to ensure defense-in-depth security against path traversal vulnerabilities.
+//
+// Example:
+//
+// cleanPath, err := fileutil.ValidateAbsolutePath(userInputPath)
+//
+//	if err != nil {
+//	   return fmt.Errorf("invalid path: %w", err)
+//	}
+//
+// content, err := os.ReadFile(cleanPath)
+func ValidateAbsolutePath(path string) (string, error) {
+	// Check for empty path
+	if path == "" {
+		return "", fmt.Errorf("path cannot be empty")
+	}
+
+	// Sanitize the filepath to prevent path traversal attacks
+	cleanPath := filepath.Clean(path)
+
+	// Verify the path is absolute to prevent relative path traversal
+	if !filepath.IsAbs(cleanPath) {
+		return "", fmt.Errorf("path must be absolute, got: %s", path)
+	}
+
+	return cleanPath, nil
+}