[WIP] Add GitHub Actions artifact upload command

cmd/gha.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -112,7 +111,7 @@ func runGHAUpload(cmd *cobra.Command, args []string) error {
 
 	// List all artifacts
 	fmt.Println("📦 Fetching workflow artifacts...")
-	ctx := context.Background()
+	ctx := cmd.Context()
 	artifacts, err := collector.ListArtifacts(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to list artifacts: %w", err)
@@ -155,32 +154,34 @@ func runGHAUpload(cmd *cobra.Command, args []string) error {
 	uploadResults := make([]map[string]string, 0, len(artifacts))
 
 	for i, artifact := range artifacts {
-		fmt.Printf("   [%d/%d] Uploading %s...\n", i+1, len(artifacts), artifact.Name)
-
-		// Download and extract artifact
-		artifactDir, err := collector.DownloadArtifact(ctx, artifact)
-		if err != nil {
-			fmt.Printf("      ❌ Failed to download: %v\n", err)
-			continue
-		}
-		defer os.RemoveAll(artifactDir)
+		func() {
+			fmt.Printf("   [%d/%d] Uploading %s...\n", i+1, len(artifacts), artifact.Name)
+
+			// Download and extract artifact
+			artifactDir, err := collector.DownloadArtifact(ctx, artifact)
+			if err != nil {
+				fmt.Printf("      ❌ Failed to download: %v\n", err)
+				return
+			}
+			defer os.RemoveAll(artifactDir)
 
-		// Upload to Vulnetix
-		uploadResp, err := uploader.UploadArtifact(txnResp.TxnID, artifact.Name, artifactDir)
-		if err != nil {
-			fmt.Printf("      ❌ Failed to upload: %v\n", err)
-			continue
-		}
+			// Upload to Vulnetix
+			uploadResp, err := uploader.UploadArtifact(txnResp.TxnID, artifact.Name, artifactDir)
+			if err != nil {
+				fmt.Printf("      ❌ Failed to upload: %v\n", err)
+				return
+			}
 
-		fmt.Printf("      ✅ Uploaded successfully\n")
-		fmt.Printf("         UUID: %s\n", uploadResp.UUID)
-		fmt.Printf("         Queue Path: %s\n", uploadResp.QueuePath)
+			fmt.Printf("      ✅ Uploaded successfully\n")
+			fmt.Printf("         UUID: %s\n", uploadResp.UUID)
+			fmt.Printf("         Queue Path: %s\n", uploadResp.QueuePath)
 
-		uploadResults = append(uploadResults, map[string]string{
-			"name":       artifact.Name,
-			"uuid":       uploadResp.UUID,
-			"queue_path": uploadResp.QueuePath,
-		})
+			uploadResults = append(uploadResults, map[string]string{
+				"name":       artifact.Name,
+				"uuid":       uploadResp.UUID,
+				"queue_path": uploadResp.QueuePath,
+			})
+		}()
 	}
 
 	fmt.Println()
@@ -194,10 +195,13 @@ func runGHAUpload(cmd *cobra.Command, args []string) error {
 	// Output JSON if requested
 	if ghaOutputJSON {
 		output := map[string]interface{}{
-			"txnid":    txnResp.TxnID,
+			"txnid":     txnResp.TxnID,
 			"artifacts": uploadResults,
 		}
-		jsonData, _ := json.MarshalIndent(output, "", "  ")
+		jsonData, err := json.MarshalIndent(output, "", "  ")
+		if err != nil {
+			return fmt.Errorf("failed to marshal JSON output: %w", err)
+		}
 		fmt.Println()
 		fmt.Println(string(jsonData))
 	}

internal/github/artifact.go

@@ -9,9 +9,23 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"regexp"
+	"strings"
 	"time"
 )
 
+const (
+	// maxArtifactSize is the maximum size for an artifact download (1GB)
+	maxArtifactSize = 1024 * 1024 * 1024
+	// artifactDownloadTimeout is the timeout for downloading artifacts
+	artifactDownloadTimeout = 10 * time.Minute
+)
+
+var (
+	// artifactNameRegex matches safe characters for artifact names
+	artifactNameRegex = regexp.MustCompile(`[^a-zA-Z0-9\-_\.]`)
+)
+
 // Artifact represents a GitHub Actions artifact
 type Artifact struct {
 	ID                 int64     `json:"id"`
@@ -68,11 +82,20 @@ func NewArtifactCollector(token, apiURL, repository, runID string) *ArtifactColl
 		repository: repository,
 		runID:      runID,
 		client: &http.Client{
-			Timeout: 60 * time.Second,
+			Timeout: artifactDownloadTimeout,
 		},
 	}
 }
 
+// sanitizeArtifactName sanitizes artifact names to prevent path traversal
+func sanitizeArtifactName(name string) string {
+	// Remove any path separators and special characters
+	sanitized := artifactNameRegex.ReplaceAllString(name, "_")
+	// Remove leading dots to prevent hidden files
+	sanitized = strings.TrimLeft(sanitized, ".")
+	return sanitized
+}
+
 // CollectMetadata collects metadata from GitHub Actions environment
 func CollectMetadata(artifactNames []string) *ArtifactMetadata {
 	// Collect standard GitHub Actions environment variables
@@ -162,8 +185,19 @@ func (c *ArtifactCollector) DownloadArtifact(ctx context.Context, artifact Artif
 		return "", fmt.Errorf("GitHub token is required")
 	}
 
+	// Check artifact size
+	if artifact.SizeInBytes > maxArtifactSize {
+		return "", fmt.Errorf("artifact size (%d bytes) exceeds maximum allowed size (%d bytes)", artifact.SizeInBytes, maxArtifactSize)
+	}
+
+	// Sanitize artifact name for directory pattern
+	sanitizedName := sanitizeArtifactName(artifact.Name)
+	if sanitizedName == "" {
+		sanitizedName = "artifact"
+	}
+
 	// Create temporary directory for extraction
-	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("artifact-%s-*", artifact.Name))
+	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("artifact-%s-*", sanitizedName))
 	if err != nil {
 		return "", fmt.Errorf("failed to create temp directory: %w", err)
 	}
@@ -199,7 +233,9 @@ func (c *ArtifactCollector) DownloadArtifact(ctx context.Context, artifact Artif
 		return "", fmt.Errorf("failed to create zip file: %w", err)
 	}
 
-	_, err = io.Copy(zipFile, resp.Body)
+	// Limit the reader to prevent resource exhaustion
+	limitedReader := io.LimitReader(resp.Body, maxArtifactSize)
+	_, err = io.Copy(zipFile, limitedReader)
 	zipFile.Close()
 	if err != nil {
 		os.RemoveAll(tmpDir)
@@ -226,19 +262,39 @@ func extractZip(zipPath, destDir string) error {
 	}
 	defer reader.Close()
 
+	// Clean the destination directory path for comparison
+	destDir = filepath.Clean(destDir)
+
 	for _, file := range reader.File {
+		// Prevent Zip Slip vulnerability by checking for path traversal
+		if strings.Contains(file.Name, "..") {
+			return fmt.Errorf("zip file contains potentially unsafe path: %s", file.Name)
+		}
+
+		// Join and clean the path
 		path := filepath.Join(destDir, file.Name)
+		path = filepath.Clean(path)
+
+		// Verify the resulting path is within destDir
+		if !strings.HasPrefix(path, destDir+string(os.PathSeparator)) && path != destDir {
+			return fmt.Errorf("zip file contains entry outside destination directory: %s", file.Name)
+		}
 
 		if file.FileInfo().IsDir() {
-			os.MkdirAll(path, file.Mode())
+			// Use safe permissions for directories
+			if err := os.MkdirAll(path, 0755); err != nil {
+				return err
+			}
 			continue
 		}
 
+		// Create parent directory with safe permissions
 		if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
 			return err
 		}
 
-		destFile, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode())
+		// Use safe permissions for files (0644 for regular files)
+		destFile, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
 		if err != nil {
 			return err
 		}
@@ -249,12 +305,24 @@ func extractZip(zipPath, destDir string) error {
 			return err
 		}
 
-		_, err = io.Copy(destFile, fileReader)
-		destFile.Close()
-		fileReader.Close()
-
-		if err != nil {
-			return err
+		// Copy with proper error handling
+		_, copyErr := io.Copy(destFile, fileReader)
+
+		// Close file reader first
+		readerCloseErr := fileReader.Close()
+
+		// Close destination file and check for close errors
+		destCloseErr := destFile.Close()
+
+		// Check all errors in order
+		if copyErr != nil {
+			return copyErr
+		}
+		if readerCloseErr != nil {
+			return readerCloseErr
+		}
+		if destCloseErr != nil {
+			return destCloseErr
 		}
 	}