Skip to content

[plan] Standardize path validation patterns across file operations #14858

@github-actions

Description

@github-actions

Objective

Extract the excellent path validation pattern from pkg/cli/completions.go:20 into a reusable helper function and apply it consistently across ~30 files using filepath.Join and file operations to strengthen defense-in-depth security posture.

Context

Issue #14844 identified inconsistent path validation: while completions.go demonstrates excellent path traversal protection with absolute path validation and security logging, this pattern is not consistently applied across the codebase. This creates maintenance challenges and makes it harder to audit for path traversal vulnerabilities.

Good Example to Extract

// pkg/cli/completions.go:20 - GOOD PATTERN
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)
        return ""
    }
    
    content, err := os.ReadFile(cleanPath)
    // ...
}

Approach

  1. Create Reusable Helper: Extract validation into pkg/fileutil/fileutil.go

    • Function: ValidateAbsolutePath(path string) (string, error)
    • Performs filepath.Clean and absolute path check
    • Returns cleaned path or descriptive error
  2. Refactor Existing Code: Update files to use the helper

    • pkg/cli/completions.go - Refactor to use new helper
    • pkg/cli/commands.go - Add validation to file operations
    • pkg/cli/trial_repository.go - Add validation (5+ locations)
    • pkg/cli/git.go - Standardize validation logic
    • pkg/cli/run_push.go - Replace ad-hoc validation
  3. Add Tests: Comprehensive path validation tests

    • Valid absolute paths pass
    • Relative paths rejected
    • Path traversal attempts (../../../etc/passwd) blocked
    • Edge cases (empty path, ., .., symlinks)
  4. Document Pattern: Add security guidelines to CONTRIBUTING.md

Files to Modify

  • Create: pkg/fileutil/fileutil.go - New validation helper
  • Create: pkg/fileutil/fileutil_test.go - Validation tests
  • Update: pkg/cli/completions.go - Use new helper (lines 20-30)
  • Update: pkg/cli/commands.go - Add validation (lines 222, 231)
  • Update: pkg/cli/trial_repository.go - Add validation (lines 175, 288, 296, 302, 340)
  • Update: pkg/cli/git.go - Standardize (lines 41, 114, 383)
  • Update: pkg/cli/run_push.go - Standardize (lines 32, 169, 482, 499)
  • Update: CONTRIBUTING.md - Document file operation security pattern

Acceptance Criteria

  • pkg/fileutil package created with ValidateAbsolutePath helper
  • Helper function validates absolute paths and returns cleaned path
  • Clear error messages for validation failures
  • All identified files refactored to use helper
  • Tests cover valid paths, relative paths, traversal attempts, edge cases
  • No breaking changes to existing functionality
  • Security pattern documented in CONTRIBUTING.md
  • CI passes with no new security warnings
    Related to [sergo] Sergo Report: Documentation-Security-Naming - 2026-02-10 #14844

AI generated by Plan Command for #14844

  • expires on Feb 13, 2026, 12:43 AM UTC

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions