[security-fix] Fix unhandled errors in pr_command.go cleanup operations (Alerts #399-404)

[github-actions]

Security Fix: Unhandled Errors in Temporary Directory Cleanup

Alert Numbers: #399, #400, #401, #402, #403, #404
Severity: Low (Warning)
Rule: G104 - Errors unhandled
Tool: gosec (Golang security checks)
Location: pkg/cli/pr_command.go:636, 645, 661, 670, 686, 695, 710

Vulnerability Description

The security scanner detected 7 instances in pkg/cli/pr_command.go where os.RemoveAll(tempDir/workingDir) is called without checking for errors. The G104 rule flags situations where errors from function calls are silently ignored, which can lead to:

1. Silent failures: Cleanup failures go unnoticed, potentially leaving temporary directories behind
2. Resource leaks: Uncleaned temporary directories can accumulate over time
3. Debugging difficulties: Errors during cleanup are not logged, making it harder to diagnose issues
4. Violation of Go best practices: Error values should never be silently ignored

The unhandled errors were found at:

• Line 636: When repository clone fails
• Line 645: When changing directory fails after clone
• Line 661: When repository clone fails (second location)
• Line 670: When changing directory fails after clone (second location)
• Line 686: When repository clone fails (third location)
• Line 695: When changing directory fails after clone (third location)
• Line 710: In defer cleanup function

Fix Applied

Added proper error handling for all os.RemoveAll() calls:

Before:

if err := cloneCmd.Run(); err != nil {
    os.RemoveAll(tempDir)
    return fmt.Errorf("failed to clone target repository: %w", err)
}

After:

if err := cloneCmd.Run(); err != nil {
    // Clean up temporary directory on error
    if rmErr := os.RemoveAll(tempDir); rmErr != nil && verbose {
        fmt.Fprintf(os.Stderr, "Warning: failed to clean up temporary directory %s: %v\n", tempDir, rmErr)
    }
    return fmt.Errorf("failed to clone target repository: %w", err)
}

This approach:

• Checks the error returned by os.RemoveAll()
• Logs a warning message when cleanup fails (in verbose mode)
• Maintains backward compatibility - cleanup failures don't block the error flow
• Follows Go best practices for defensive programming

Security Best Practices

1. Always handle errors: Even in cleanup/error paths, errors should be checked
2. Consistent error handling: Applied the same pattern used elsewhere in the codebase
3. Non-critical operations: Since this is cleanup in an error path, we log warnings rather than returning errors
4. Informative logging: Includes the directory path and error details for debugging

Testing Considerations

• ✅ Build succeeds without errors
• ✅ No breaking changes to existing functionality
• ✅ Error handling is consistent with cleanup operations in other files
• ✅ Minimal, surgical changes - only added error checking

Impact Assessment

Risk: Minimal
Breaking Changes: None
Backwards Compatibility: Full
Performance: No impact

Files Modified

• pkg/cli/pr_command.go:
  • Lines 636-665: Added error handling for cleanup after clone failure
  • Lines 645-678: Added error handling for cleanup after chdir failure
  • Lines 661-697: Added error handling for cleanup after clone failure (second location)
  • Lines 670-709: Added error handling for cleanup after chdir failure (second location)
  • Lines 686-697: Added error handling for cleanup after clone failure (third location)
  • Lines 695-709: Added error handling for cleanup after chdir failure (third location)
  • Lines 710-727: Added error handling for cleanup in defer function

References

• CodeQL Alerts:
  • https://github.com/githubnext/gh-aw/security/code-scanning/399
  • https://github.com/githubnext/gh-aw/security/code-scanning/400
  • https://github.com/githubnext/gh-aw/security/code-scanning/401
  • https://github.com/githubnext/gh-aw/security/code-scanning/402
  • https://github.com/githubnext/gh-aw/security/code-scanning/403
  • https://github.com/githubnext/gh-aw/security/code-scanning/404
• Gosec G104: Audit errors not checked
• Go Error Handling: [Effective Go - Errors]((redacted)

---

🤖 Generated by Security Fix Agent in workflow run 20556246680

AI generated by Security Fix PR