feat: add config for CI linting and code quality improvements

.golangci.yml

@@ -0,0 +1,22 @@
+version: "2"
+
+run:
+  timeout: 5m
+  modules-download-mode: readonly
+
+linters:
+  enable:
+    - errcheck
+    - govet
+    - staticcheck
+    - misspell
+    - gocritic
+    - revive
+    - ineffassign
+    - unused
+    - gocyclo
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+

Makefile

@@ -1,5 +1,5 @@
 PROJECT := tar-diff
-VERSION := $(shell grep -oP 'VERSION\s*=\s*"\K[^"]+' pkg/common/version.go)
+VERSION := $(shell grep -oP 'VERSION\s*=\s*"\K[^"]+' pkg/protocol/version.go)
 PROJ_TARBALL := $(PROJECT)_$(VERSION).tar.gz
 
 .PHONY: all build clean fmt install lint test tools dist unit-test integration-test validate .install.golangci-lint
@@ -69,7 +69,11 @@ fmt:
 
 validate: lint
 	@go vet $(GOFLAGS) ./...
-	@test -z "$$(gofmt -s -l . | tee /dev/stderr)"
+	@if [ -n "$$(gofmt -s -l .)" ]; then \
+		echo "Files need formatting:"; \
+		gofmt -s -l .; \
+		exit 1; \
+	fi
 
 lint:
 	GOFLAGS=$(GOFLAGS) $(GOBIN)/golangci-lint run

cmd/tar-diff/main.go

@@ -1,3 +1,4 @@
+// Package main implements the tar-diff command line tool for creating binary diffs between tar archives.
 package main
 
 import (
@@ -7,8 +8,8 @@ import (
 	"os"
 	"path"
 
-	"github.com/containers/tar-diff/pkg/common"
-	tar_diff "github.com/containers/tar-diff/pkg/tar-diff"
+	"github.com/containers/tar-diff/pkg/protocol"
+	tardiff "github.com/containers/tar-diff/pkg/tar-diff"
 )
 
 var version = flag.Bool("version", false, "Show version")
@@ -26,7 +27,7 @@ func main() {
 	flag.Parse()
 
 	if *version {
-		fmt.Printf("%s %s\n", path.Base(os.Args[0]), common.VERSION)
+		fmt.Printf("%s %s\n", path.Base(os.Args[0]), protocol.VERSION)
 		return
 	}
 
@@ -44,39 +45,44 @@ func main() {
 		log.Fatalf("Error: %s", err)
 	}
 
+	newFile, err := os.Open(newFilename)
+	if err != nil {
+		_ = oldFile.Close() // Clean up first file before exiting
+		log.Fatalf("Error: %s", err)
+	}
+
+	deltaFile, err := os.Create(deltaFilename)
+	if err != nil {
+		_ = oldFile.Close()
+		_ = newFile.Close()
+		log.Fatalf("Error: %s", err)
+	}
+
+	// Set up deferred cleanup after all files are successfully opened
 	defer func() {
 		if err := oldFile.Close(); err != nil {
 			fmt.Fprintf(os.Stderr, "Error closing %s: %s\n", oldFilename, err)
 		}
 	}()
-
-	newFile, err := os.Open(newFilename)
-	if err != nil {
-		log.Fatalf("Error: %s", err)
-	}
 	defer func() {
 		if err := newFile.Close(); err != nil {
 			fmt.Fprintf(os.Stderr, "Error closing %s: %s\n", newFilename, err)
 		}
 	}()
+	defer func() {
+		if err := deltaFile.Close(); err != nil {
+			fmt.Fprintf(os.Stderr, "Error closing %s: %s\n", deltaFilename, err)
+		}
+	}()
 
-	deltaFile, err := os.Create(deltaFilename)
-	if err != nil {
-		log.Fatalf("Error: %s", err)
-	}
-
-	options := tar_diff.NewOptions()
+	options := tardiff.NewOptions()
 	options.SetCompressionLevel(*compressionLevel)
 	options.SetMaxBsdiffFileSize(int64(*maxBsdiffSize) * 1024 * 1024)
 
-	err = tar_diff.Diff(oldFile, newFile, deltaFile, options)
+	err = tardiff.Diff(oldFile, newFile, deltaFile, options)
 	if err != nil {
-		log.Fatalf("Error: %s", err)
-	}
-
-	err = deltaFile.Close()
-	if err != nil {
-		log.Fatalf("Error: %s", err)
+		log.Printf("Error: %s", err)
+		return
 	}
 
 }

cmd/tar-patch/main.go

@@ -1,3 +1,4 @@
+// Package main implements the tar-patch command line tool for applying binary diffs to tar archives.
 package main
 
 import (
@@ -7,8 +8,8 @@ import (
 	"os"
 	"path"
 
-	"github.com/containers/tar-diff/pkg/common"
-	tar_patch "github.com/containers/tar-diff/pkg/tar-patch"
+	"github.com/containers/tar-diff/pkg/protocol"
+	tarpatch "github.com/containers/tar-diff/pkg/tar-patch"
 )
 
 var version = flag.Bool("version", false, "Show version")
@@ -23,7 +24,7 @@ func main() {
 	flag.Parse()
 
 	if *version {
-		fmt.Printf("%s %s\n", path.Base(os.Args[0]), common.VERSION)
+		fmt.Printf("%s %s\n", path.Base(os.Args[0]), protocol.VERSION)
 		return
 	}
 
@@ -36,17 +37,20 @@ func main() {
 	extractedDir := flag.Arg(1)
 	patchedFilename := flag.Arg(2)
 
-	dataSource := tar_patch.NewFilesystemDataSource(extractedDir)
-	defer func() {
-		if err := dataSource.Close(); err != nil {
-			fmt.Fprintf(os.Stderr, "Error closing %s: %s\n", extractedDir, err)
-		}
-	}()
+	dataSource := tarpatch.NewFilesystemDataSource(extractedDir)
 
 	deltaFile, err := os.Open(deltaFilename)
 	if err != nil {
+		_ = dataSource.Close() // Clean up data source before exiting
 		log.Fatalf("Unable to open %s: %s", deltaFilename, err)
 	}
+
+	// Set up deferred cleanup after all resources are successfully opened
+	defer func() {
+		if err := dataSource.Close(); err != nil {
+			fmt.Fprintf(os.Stderr, "Error closing %s: %s\n", extractedDir, err)
+		}
+	}()
 	defer func() {
 		if err := deltaFile.Close(); err != nil {
 			fmt.Fprintf(os.Stderr, "Error closing %s: %s\n", deltaFilename, err)
@@ -61,7 +65,8 @@ func main() {
 		var err error
 		patchedFile, err = os.Create(patchedFilename)
 		if err != nil {
-			log.Fatalf("Unable to create %s: %s", patchedFilename, err)
+			log.Printf("Unable to create %s: %s", patchedFilename, err)
+			return
 		}
 		defer func() {
 			if err := patchedFile.Close(); err != nil {
@@ -70,8 +75,9 @@ func main() {
 		}()
 	}
 
-	err = tar_patch.Apply(deltaFile, dataSource, patchedFile)
+	err = tarpatch.Apply(deltaFile, dataSource, patchedFile)
 	if err != nil {
-		log.Fatalf("Error applying diff: %s", err)
+		log.Printf("Error applying diff: %s", err)
+		return
 	}
 }

pkg/protocol/common.go

@@ -0,0 +1,43 @@
+// Package protocol provides shared constants and data structures for tar-diff operations.
+package protocol
+
+import "path/filepath"
+
+// Delta operation constants define the types of operations in a delta file.
+const (
+	DeltaOpData    = iota // Raw data operation
+	DeltaOpOpen    = iota // Open file operation
+	DeltaOpCopy    = iota // Copy from source operation
+	DeltaOpAddData = iota // Add new data operation
+	DeltaOpSeek    = iota // Seek operation
+)
+
+// DeltaHeader is the magic header bytes for tar-diff files.
+var DeltaHeader = [...]byte{'t', 'a', 'r', 'd', 'f', '1', '\n', 0}
+
+// CleanPath cleans up the path lexically and prevents path traversal attacks.
+// Any ".." that extends outside the first elements (or the root itself) is invalid and returns "".
+// Uses filepath.Clean for proper cross-platform path handling (Windows backslashes, drive letters).
+// This is a security-critical function used by both tar-diff and tar-patch packages.
+func CleanPath(pathName string) string {
+	// A path with a volume name is absolute and can lead to path traversal on Windows.
+	if filepath.VolumeName(pathName) != "" {
+		return ""
+	}
+
+	// Convert to forward slashes for consistent processing
+	pathName = filepath.ToSlash(pathName)
+
+	// We make the path always absolute, that way filepath.Clean() ensures it never goes outside the top ("root") dir
+	// even if its a relative path
+	clean := filepath.Clean(filepath.Join("/", pathName))
+
+	// Convert back to forward slashes to ensure consistent output
+	clean = filepath.ToSlash(clean)
+
+	// We clean the initial slash, making all result relative (or "" which is error)
+	if len(clean) > 0 && clean[0] == '/' {
+		return clean[1:]
+	}
+	return ""
+}

pkg/protocol/version.go

@@ -0,0 +1,4 @@
+package protocol
+
+// VERSION contains the current version string for the tar-diff tool.
+var VERSION = "v0.1.2"

pkg/tar-diff/analysis.go

@@ -1,4 +1,5 @@
-package tar_diff
+// Package tardiff provides functionality for analyzing and creating binary differences between tar archives.
+package tardiff
 
 import (
 	"archive/tar"
@@ -11,6 +12,7 @@ import (
 	"strings"
 
 	"github.com/containers/image/v5/pkg/compression"
+	"github.com/containers/tar-diff/pkg/protocol"
 )
 
 type tarFileInfo struct {
@@ -80,14 +82,6 @@ func isSparseFile(hdr *tar.Header) bool {
 
 // Cleans up the path lexically
 // Any ".." that extends outside the first elements (or the root itself) is invalid and returns ""
-func cleanPath(pathName string) string {
-	// We make the path always absolute, that way path.Clean() ensure it never goes outside the top ("root") dir
-	// even if its a relative path
-	clean := path.Clean("/" + pathName)
-
-	// We clean the initial slash, making all result relative (or "" which is error)
-	return clean[1:]
-}
 
 // Ignore all the files that make no sense to either delta or re-use as is
 func useTarFile(hdr *tar.Header, cleanPath string) bool {
@@ -143,16 +137,15 @@ func analyzeTar(tarMaybeCompressed io.Reader) (*tarInfo, error) {
 		if err != nil {
 			if err == io.EOF {
 				break // Expected error
-			} else {
-				return nil, err
 			}
+			return nil, err
 		}
 		// Normalize name, for safety
-		pathname := cleanPath(hdr.Name)
+		pathname := protocol.CleanPath(hdr.Name)
 
 		// Handle hardlinks
 		if hdr.Typeflag == tar.TypeLink {
-			linkname := cleanPath(hdr.Linkname)
+			linkname := protocol.CleanPath(hdr.Linkname)
 			if linkname != "" {
 				// Store a copy of the header for later use
 				hdrCopy := *hdr
@@ -216,11 +209,10 @@ func isDeltaCandidate(file *tarFileInfo) bool {
 func nameIsSimilar(a *tarFileInfo, b *tarFileInfo, fuzzy int) bool {
 	if fuzzy == 0 {
 		return a.basename == b.basename
-	} else {
-		aa := strings.SplitAfterN(a.basename, ".", 2)[0]
-		bb := strings.SplitAfterN(b.basename, ".", 2)[0]
-		return aa == bb
 	}
+	aa := strings.SplitAfterN(a.basename, ".", 2)[0]
+	bb := strings.SplitAfterN(b.basename, ".", 2)[0]
+	return aa == bb
 }
 
 // Check that two files are not wildly dissimilar in size.
@@ -257,9 +249,8 @@ func extractDeltaData(tarMaybeCompressed io.Reader, sourceByIndex map[int]*sourc
 		if err != nil {
 			if err == io.EOF {
 				break // Expected error
-			} else {
-				return err
 			}
+			return err
 		}
 		info := sourceByIndex[index]
 		if info != nil && info.usedForDelta {
@@ -279,7 +270,7 @@ func abs(n int64) int64 {
 	}
 	return n
 }
-func analyzeForDelta(old *tarInfo, new *tarInfo, oldFile io.Reader) (*deltaAnalysis, error) {
+func analyzeForDelta(old *tarInfo, newTar *tarInfo, oldFile io.Reader) (*deltaAnalysis, error) {
 	sourceInfos := make([]sourceInfo, 0, len(old.files))
 	for i := range old.files {
 		sourceInfos = append(sourceInfos, sourceInfo{file: &old.files[i]})
@@ -297,10 +288,10 @@ func analyzeForDelta(old *tarInfo, new *tarInfo, oldFile io.Reader) (*deltaAnaly
 		}
 	}
 
-	targetInfos := make([]targetInfo, 0, len(new.files)+len(new.hardlinks))
+	targetInfos := make([]targetInfo, 0, len(newTar.files)+len(newTar.hardlinks))
 
-	for i := range new.files {
-		file := &new.files[i]
+	for i := range newTar.files {
+		file := &newTar.files[i]
 		// First look for exact content match
 		usedForDelta := false
 		var source *sourceInfo
@@ -359,14 +350,14 @@ func analyzeForDelta(old *tarInfo, new *tarInfo, oldFile io.Reader) (*deltaAnaly
 		targetInfos = append(targetInfos, info)
 	}
 
-	targetInfoByIndex := make(map[int]*targetInfo, len(new.files)+len(new.hardlinks))
+	targetInfoByIndex := make(map[int]*targetInfo, len(newTar.files)+len(newTar.hardlinks))
 	for i := range targetInfos {
 		t := &targetInfos[i]
 		targetInfoByIndex[t.file.index] = t
 	}
 	// Add hardlinks to targetInfoByIndex
-	for i := range new.hardlinks {
-		hl := &new.hardlinks[i]
+	for i := range newTar.hardlinks {
+		hl := &newTar.hardlinks[i]
 		info := targetInfo{hardlink: hl}
 		targetInfos = append(targetInfos, info)
 		targetInfoByIndex[hl.index] = &targetInfos[len(targetInfos)-1]

pkg/tar-diff/analysis_test.go

@@ -1,4 +1,4 @@
-package tar_diff
+package tardiff
 
 import (
 	"archive/tar"