Merge pull request #17 from airframesio/copilot/sub-pr-4

Fix golangci-lint issues and add input validation to prevent security vulnerabilities

Author: kevinelliott

cmd/archiver.go

@@ -3,10 +3,11 @@ package cmd
 import (
 	"bytes"
 	"context"
-	"crypto/md5"
+	"crypto/md5" //nolint:gosec // MD5 used for checksums, not cryptography
 	"database/sql"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -20,7 +21,12 @@ import (
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/lipgloss"
 	"github.com/klauspost/compress/zstd"
-	_ "github.com/lib/pq"
+	"github.com/lib/pq"
+)
+
+// Stage constants
+const (
+	StageSkipped = "Skipped"
 )
 
 type Archiver struct {
@@ -175,12 +181,12 @@ func (a *Archiver) checkTablePermissions(ctx context.Context) error {
 	`
 
 	err := a.db.QueryRowContext(ctx, checkPermissionQuery, a.config.Table, a.config.Table).Scan(&hasPermission)
-	if err != nil && err != sql.ErrNoRows {
+	if err != nil && !errors.Is(err, sql.ErrNoRows) {
 		return fmt.Errorf("failed to check table permissions: %w", err)
 	}
 
 	// If the base table exists and we don't have permission, fail
-	if err != sql.ErrNoRows && !hasPermission {
+	if !errors.Is(err, sql.ErrNoRows) && !hasPermission {
 		return fmt.Errorf("insufficient permissions to read table '%s'", a.config.Table)
 	}
 
@@ -197,13 +203,13 @@ func (a *Archiver) checkTablePermissions(ctx context.Context) error {
 
 	var samplePartition string
 	err = a.db.QueryRowContext(ctx, partitionCheckQuery, pattern).Scan(&samplePartition)
-	if err != nil && err != sql.ErrNoRows {
+	if err != nil && !errors.Is(err, sql.ErrNoRows) {
 		// Only fail if it's not a "no rows" error
 		return fmt.Errorf("failed to check partition table permissions: %w", err)
 	}
 
 	// Check if we found any partitions at all (with or without permissions)
-	if err == sql.ErrNoRows {
+	if errors.Is(err, sql.ErrNoRows) {
 		// Let's see if partitions exist but we can't access them
 		var partitionExists bool
 		existsQuery := `
@@ -311,7 +317,7 @@ func (a *Archiver) discoverPartitionsWithUI(program *tea.Program) ([]PartitionIn
 			// Update count progress
 			program.Send(updateCount(i+1, len(matchingTables), table.name))
 
-			countQuery := fmt.Sprintf("SELECT COUNT(*) FROM %s", table.name)
+			countQuery := fmt.Sprintf("SELECT COUNT(*) FROM %s", pq.QuoteIdentifier(table.name))
 			var count int64
 			if err := a.db.QueryRow(countQuery).Scan(&count); err == nil {
 				partitions = append(partitions, PartitionInfo{
@@ -420,7 +426,7 @@ func (a *Archiver) discoverPartitions() ([]PartitionInfo, error) {
 				len(matchingTables),
 				table.name)
 
-			countQuery := fmt.Sprintf("SELECT COUNT(*) FROM %s", table.name)
+			countQuery := fmt.Sprintf("SELECT COUNT(*) FROM %s", pq.QuoteIdentifier(table.name))
 			var count int64
 			if err := a.db.QueryRow(countQuery).Scan(&count); err == nil {
 				partitions = append(partitions, PartitionInfo{
@@ -551,7 +557,7 @@ func (a *Archiver) ProcessPartitionWithProgress(partition PartitionInfo, index i
 					// Cached metadata matches S3 - skip without extraction
 					result.Skipped = true
 					result.SkipReason = fmt.Sprintf("Cached metadata matches S3 (size=%d, md5=%s)", cachedSize, cachedMD5)
-					result.Stage = "Skipped"
+					result.Stage = StageSkipped
 					result.BytesWritten = cachedSize
 					if a.config.Debug {
 						fmt.Printf("     ✅ Skipping based on cache: Size and MD5 match\n")
@@ -620,7 +626,7 @@ func (a *Archiver) ProcessPartitionWithProgress(partition PartitionInfo, index i
 	}
 
 	// Calculate MD5 hash of compressed data
-	hasher := md5.New()
+	hasher := md5.New() //nolint:gosec // MD5 used for checksums, not cryptography
 	hasher.Write(compressed)
 	localMD5 := hex.EncodeToString(hasher.Sum(nil))
 	localSize := int64(len(compressed))
@@ -647,7 +653,7 @@ func (a *Archiver) ProcessPartitionWithProgress(partition PartitionInfo, index i
 				// Single-part upload with matching MD5
 				result.Skipped = true
 				result.SkipReason = fmt.Sprintf("Already exists with matching size (%d bytes) and MD5 (%s)", s3Size, s3ETag)
-				result.Stage = "Skipped"
+				result.Stage = StageSkipped
 				if a.config.Debug {
 					fmt.Printf("     ✅ Skipping: Size and MD5 match\n")
 				}
@@ -664,7 +670,7 @@ func (a *Archiver) ProcessPartitionWithProgress(partition PartitionInfo, index i
 				if s3ETag == localMultipartETag {
 					result.Skipped = true
 					result.SkipReason = fmt.Sprintf("Already exists with matching size (%d bytes) and multipart ETag (%s)", s3Size, s3ETag)
-					result.Stage = "Skipped"
+					result.Stage = StageSkipped
 					if a.config.Debug {
 						fmt.Printf("     ✅ Skipping: Size and multipart ETag match\n")
 					}
@@ -718,7 +724,9 @@ func (a *Archiver) ProcessPartitionWithProgress(partition PartitionInfo, index i
 }
 
 func (a *Archiver) extractDataWithProgress(partition PartitionInfo, program *tea.Program) ([]byte, error) {
-	query := fmt.Sprintf("SELECT row_to_json(t) FROM %s t", partition.TableName)
+	// Use pq.QuoteIdentifier to safely quote the table name
+	quotedTable := pq.QuoteIdentifier(partition.TableName)
+	query := fmt.Sprintf("SELECT row_to_json(t) FROM %s t", quotedTable) //nolint:gosec // Table name is quoted with pq.QuoteIdentifier
 
 	rows, err := a.db.Query(query)
 	if err != nil {
@@ -763,6 +771,11 @@ func (a *Archiver) extractDataWithProgress(partition PartitionInfo, program *tea
 		}
 	}
 
+	// Check for errors from iterating over rows
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
 	// Final update
 	if program != nil {
 		if partition.RowCount > 0 {
@@ -848,7 +861,7 @@ func (a *Archiver) calculateMultipartETag(data []byte) string {
 
 	// If it would be a single part, just return regular MD5
 	if numParts == 1 {
-		hasher := md5.New()
+		hasher := md5.New() //nolint:gosec // MD5 used for checksums, not cryptography
 		hasher.Write(data)
 		return hex.EncodeToString(hasher.Sum(nil))
 	}
@@ -862,13 +875,13 @@ func (a *Archiver) calculateMultipartETag(data []byte) string {
 			end = len(data)
 		}
 
-		partHasher := md5.New()
+		partHasher := md5.New() //nolint:gosec // MD5 used for checksums, not cryptography
 		partHasher.Write(data[start:end])
 		partMD5s = append(partMD5s, partHasher.Sum(nil)...)
 	}
 
 	// Calculate MD5 of concatenated MD5s
-	finalHasher := md5.New()
+	finalHasher := md5.New() //nolint:gosec // MD5 used for checksums, not cryptography
 	finalHasher.Write(partMD5s)
 	finalMD5 := hex.EncodeToString(finalHasher.Sum(nil))
 
@@ -894,18 +907,18 @@ func (a *Archiver) uploadToS3(key string, data []byte) error {
 
 		_, err := a.s3Uploader.Upload(uploadInput)
 		return err
-	} else {
-		// Use simple PutObject for smaller files
-		putInput := &s3.PutObjectInput{
-			Bucket:      aws.String(a.config.S3.Bucket),
-			Key:         aws.String(key),
-			Body:        bytes.NewReader(data),
-			ContentType: aws.String("application/zstd"),
-		}
+	}
 
-		_, err := a.s3Client.PutObject(putInput)
-		return err
+	// Use simple PutObject for smaller files
+	putInput := &s3.PutObjectInput{
+		Bucket:      aws.String(a.config.S3.Bucket),
+		Key:         aws.String(key),
+		Body:        bytes.NewReader(data),
+		ContentType: aws.String("application/zstd"),
 	}
+
+	_, err := a.s3Client.PutObject(putInput)
+	return err
 }
 
 func (a *Archiver) printSummary(results []ProcessResult) {

cmd/cache.go

@@ -47,7 +47,7 @@ type RowCountEntry struct {
 func getCachePath(tableName string) string {
 	homeDir, _ := os.UserHomeDir()
 	cacheDir := filepath.Join(homeDir, ".postgresql-archiver", "cache")
-	_ = os.MkdirAll(cacheDir, 0755)
+	_ = os.MkdirAll(cacheDir, 0o755)
 	return filepath.Join(cacheDir, fmt.Sprintf("%s_metadata.json", tableName))
 }
 
@@ -155,7 +155,7 @@ func (c *PartitionCache) save(tableName string) error {
 		return err
 	}
 
-	return os.WriteFile(cachePath, data, 0644)
+	return os.WriteFile(cachePath, data, 0o600)
 }
 
 // Backward compatibility wrapper - kept for potential future use

cmd/cache_server.go

@@ -19,7 +19,7 @@ import (
 var (
 	serverPort int
 	upgrader   = websocket.Upgrader{
-		CheckOrigin: func(r *http.Request) bool {
+		CheckOrigin: func(_ *http.Request) bool {
 			return true // Allow all origins for local development
 		},
 	}
@@ -91,7 +91,7 @@ func startBackgroundServices() {
 	})
 }
 
-func runCacheServer(cmd *cobra.Command, args []string) error {
+func runCacheServer(_ *cobra.Command, _ []string) error {
 	// Set up HTTP routes
 	http.HandleFunc("/", serveCacheViewer)
 	http.HandleFunc("/api/cache", serveCacheData)
@@ -107,16 +107,22 @@ func runCacheServer(cmd *cobra.Command, args []string) error {
 	fmt.Printf("🌐 Open your browser to view cache data\n")
 	fmt.Printf("⌨️  Press Ctrl+C to stop the server\n\n")
 
-	return http.ListenAndServe(addr, nil)
+	server := &http.Server{
+		Addr:              addr,
+		ReadHeaderTimeout: 10 * time.Second,
+		ReadTimeout:       30 * time.Second,
+		WriteTimeout:      30 * time.Second,
+	}
+	return server.ListenAndServe()
 }
 
-func serveCacheViewer(w http.ResponseWriter, r *http.Request) {
+func serveCacheViewer(w http.ResponseWriter, _ *http.Request) {
 	html := getCacheViewerHTML()
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	_, _ = w.Write([]byte(html))
 }
 
-func serveCacheData(w http.ResponseWriter, r *http.Request) {
+func serveCacheData(w http.ResponseWriter, _ *http.Request) {
 	// Enable CORS for local development
 	w.Header().Set("Access-Control-Allow-Origin", "*")
 	w.Header().Set("Content-Type", "application/json")
@@ -215,7 +221,7 @@ func serveCacheData(w http.ResponseWriter, r *http.Request) {
 	_ = json.NewEncoder(w).Encode(response)
 }
 
-func serveStatusData(w http.ResponseWriter, r *http.Request) {
+func serveStatusData(w http.ResponseWriter, _ *http.Request) {
 	// Enable CORS for local development
 	w.Header().Set("Access-Control-Allow-Origin", "*")
 	w.Header().Set("Content-Type", "application/json")
@@ -321,8 +327,8 @@ func dataMonitor() {
 	taskDir := filepath.Join(homeDir, ".postgresql-archiver")
 
 	// Create directories if they don't exist
-	_ = os.MkdirAll(cacheDir, 0755)
-	_ = os.MkdirAll(taskDir, 0755)
+	_ = os.MkdirAll(cacheDir, 0o755)
+	_ = os.MkdirAll(taskDir, 0o755)
 
 	// Watch cache directory
 	if err := watcher.Add(cacheDir); err != nil {

cmd/cache_test.go

@@ -8,6 +8,8 @@ import (
 	"time"
 )
 
+const testTablePartition = "test_table_20240101"
+
 func TestPartitionCache(t *testing.T) {
 	// Create a temporary directory for test cache
 	tempDir, err := os.MkdirTemp("", "cache_test")
@@ -41,7 +43,7 @@ func TestPartitionCache(t *testing.T) {
 			Entries: make(map[string]PartitionCacheEntry),
 		}
 
-		partition := "test_table_20240101"
+		partition := testTablePartition
 		count := int64(1000)
 		cache.setRowCount(partition, count)
 
@@ -68,7 +70,7 @@ func TestPartitionCache(t *testing.T) {
 			Entries: make(map[string]PartitionCacheEntry),
 		}
 
-		partition := "test_table_20240101"
+		partition := testTablePartition
 		s3Key := "export/test_table/2024/01/2024-01-01.jsonl.zst"
 		compressedSize := int64(1024)
 		uncompressedSize := int64(5120)
@@ -103,7 +105,7 @@ func TestPartitionCache(t *testing.T) {
 			Entries: make(map[string]PartitionCacheEntry),
 		}
 
-		partition := "test_table_20240101"
+		partition := testTablePartition
 		s3Key := "export/test_table/2024/01/2024-01-01.jsonl.zst"
 
 		// Set file metadata first
@@ -133,7 +135,7 @@ func TestPartitionCache(t *testing.T) {
 			Entries: make(map[string]PartitionCacheEntry),
 		}
 
-		partition := "test_table_20240101"
+		partition := testTablePartition
 		errMsg := "connection failed"
 
 		cache.setError(partition, errMsg)
@@ -243,11 +245,11 @@ func TestPartitionCache(t *testing.T) {
 
 		// Save legacy cache
 		cacheDir := filepath.Join(tempDir, ".postgresql-archiver", "cache")
-		_ = os.MkdirAll(cacheDir, 0755)
+		_ = os.MkdirAll(cacheDir, 0o755)
 		legacyPath := filepath.Join(cacheDir, "legacy_table_counts.json")
 
 		data, _ := json.MarshalIndent(legacyCache, "", "  ")
-		_ = os.WriteFile(legacyPath, data, 0644)
+		_ = os.WriteFile(legacyPath, data, 0o644)
 
 		// Load should migrate
 		cache, err := loadPartitionCache("legacy_table")