Update golangci-lint to v2.12.2 with canonical config (closes #60)

Adopts golangci-lint v2.12.2 and the canonical .golangci.yml (default: all), and fixes all resulting findings across the tree.

Two intended behavior changes: absent MFFilePath.Mtime is handled explicitly in freshen, list and export rather than dereferenced (main panicked); gpg positional key IDs now follow an explicit -- end-of-options marker.

All twelve reworded user-visible error messages restored to byte-identical parity with main and pinned by tests.

internal/cli/fetch.go

@@ -2,7 +2,9 @@ package cli
 
 import (
 	"bytes"
+	"context"
 	"crypto/sha256"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -20,6 +22,45 @@ import (
 	"sneak.berlin/go/mfer/mfer"
 )
 
+const (
+	// progressChanBuffer is the buffer size of the download progress
+	// channel.
+	progressChanBuffer = 10
+
+	// bitsPerByte converts a bytes-per-second rate to bits per second.
+	bitsPerByte = 8
+
+	// dirPerms is the permission mode for directories created for
+	// downloaded files. Fetched trees are content that is normally
+	// published (served by a web server, read by another uid), so the
+	// traversal bit for group and other must stay set.
+	dirPerms os.FileMode = 0o755
+
+	// Bitrate unit thresholds in bits per second.
+	bpsPerGbps = 1e9
+	bpsPerMbps = 1e6
+	bpsPerKbps = 1e3
+)
+
+var (
+	// errURLRequired indicates the fetch command was run without a URL
+	// argument.
+	errURLRequired = errors.New("URL argument required")
+	// errEmptyPath indicates an empty file path in the manifest.
+	errEmptyPath = errors.New("empty path")
+	// errAbsolutePath indicates an absolute file path in the manifest.
+	errAbsolutePath = errors.New("absolute path not allowed")
+	// errPathTraversal indicates a manifest path escaping the target
+	// directory.
+	errPathTraversal = errors.New("path traversal not allowed")
+	// errSizeMismatch indicates a downloaded file with an unexpected
+	// size.
+	errSizeMismatch = errors.New("size mismatch")
+	// errHashMismatch indicates a downloaded file whose hash matches no
+	// manifest hash.
+	errHashMismatch = errors.New("hash mismatch")
+)
+
 // DownloadProgress reports the progress of a single file download.
 type DownloadProgress struct {
 	Path        string        // File path being downloaded
@@ -29,14 +70,98 @@ type DownloadProgress struct {
 	ETA         time.Duration // Estimated time to completion
 }
 
+// httpGet issues a GET request for the given URL using the provided
+// context and returns the response. The caller must close the body.
+//
+// Errors are returned unwrapped: this helper replaced direct http.Get
+// calls, and each caller already supplies its own context string, so
+// adding one here would change user-visible messages.
+func httpGet(ctx context.Context, fileURL string) (*http.Response, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, fileURL, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return nil, err
+	}
+
+	return resp, nil
+}
+
+// reportDownloadProgress renders download progress until the channel
+// closes, then closes done.
+func reportDownloadProgress(progress <-chan DownloadProgress, done chan<- struct{}) {
+	defer close(done)
+
+	for p := range progress {
+		rate := formatBitrate(p.BytesPerSec * bitsPerByte)
+		if p.ETA > 0 {
+			log.Infof("%s: %s/%s, %s, ETA %s",
+				p.Path, humanize.IBytes(safeUint64(p.BytesRead)),
+				humanize.IBytes(safeUint64(p.TotalBytes)),
+				rate, p.ETA.Round(time.Second))
+		} else {
+			log.Infof("%s: %s/%s, %s",
+				p.Path, humanize.IBytes(safeUint64(p.BytesRead)),
+				humanize.IBytes(safeUint64(p.TotalBytes)), rate)
+		}
+	}
+}
+
+// manifestBaseURL returns the URL of the directory containing the
+// manifest, with a trailing slash.
+func manifestBaseURL(manifestURL string) (*url.URL, error) {
+	baseURL, err := url.Parse(manifestURL)
+	if err != nil {
+		return nil, fmt.Errorf("fetch: invalid manifest URL: %w", err)
+	}
+
+	baseURL.Path = path.Dir(baseURL.Path)
+	if !strings.HasSuffix(baseURL.Path, "/") {
+		baseURL.Path += "/"
+	}
+
+	return baseURL, nil
+}
+
+// downloadManifestFiles downloads every file in the manifest, reporting
+// progress on the progress channel.
+func downloadManifestFiles(
+	ctx context.Context,
+	baseURL *url.URL,
+	files []*mfer.MFFilePath,
+	progress chan<- DownloadProgress,
+) error {
+	for _, f := range files {
+		// Sanitize the path to prevent path traversal attacks
+		localPath, err := sanitizePath(f.GetPath())
+		if err != nil {
+			return fmt.Errorf("invalid path in manifest: %w", err)
+		}
+
+		fileURL := baseURL.String() + encodeFilePath(f.GetPath())
+		log.Infof("fetching %s", f.GetPath())
+
+		err = downloadFile(ctx, fileURL, localPath, f, progress)
+		if err != nil {
+			return fmt.Errorf("failed to download %s: %w", f.GetPath(), err)
+		}
+	}
+
+	return nil
+}
+
 func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
 	log.Debug("fetchManifestOperation()")
 
 	if ctx.Args().Len() == 0 {
-		return fmt.Errorf("URL argument required")
+		return errURLRequired
 	}
 
 	inputURL := ctx.Args().Get(0)
+
 	manifestURL, err := resolveManifestURL(inputURL)
 	if err != nil {
 		return fmt.Errorf("invalid URL: %w", err)
@@ -45,14 +170,16 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
 	log.Infof("fetching manifest from %s", manifestURL)
 
 	// Fetch manifest
-	resp, err := http.Get(manifestURL)
+	resp, err := httpGet(ctx.Context, manifestURL)
 	if err != nil {
 		return fmt.Errorf("failed to fetch manifest: %w", err)
 	}
+
 	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode)
+		return fmt.Errorf("failed to fetch manifest: %w %d",
+			errHTTPStatus, resp.StatusCode)
 	}
 
 	// Parse manifest
@@ -65,74 +192,43 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
 	log.Infof("manifest contains %d files", len(files))
 
 	// Compute base URL (directory containing manifest)
-	baseURL, err := url.Parse(manifestURL)
+	baseURL, err := manifestBaseURL(manifestURL)
 	if err != nil {
-		return fmt.Errorf("fetch: invalid manifest URL: %w", err)
-	}
-	baseURL.Path = path.Dir(baseURL.Path)
-	if !strings.HasSuffix(baseURL.Path, "/") {
-		baseURL.Path += "/"
+		return err
 	}
 
 	// Calculate total bytes to download
 	var totalBytes int64
 	for _, f := range files {
-		totalBytes += f.Size
+		totalBytes += f.GetSize()
 	}
 
-	// Create progress channel
-	progress := make(chan DownloadProgress, 10)
-
-	// Start progress reporter goroutine
+	// Create progress channel and start progress reporter goroutine
+	progress := make(chan DownloadProgress, progressChanBuffer)
 	done := make(chan struct{})
-	go func() {
-		defer close(done)
-		for p := range progress {
-			rate := formatBitrate(p.BytesPerSec * 8)
-			if p.ETA > 0 {
-				log.Infof("%s: %s/%s, %s, ETA %s",
-					p.Path, humanize.IBytes(uint64(p.BytesRead)), humanize.IBytes(uint64(p.TotalBytes)),
-					rate, p.ETA.Round(time.Second))
-			} else {
-				log.Infof("%s: %s/%s, %s",
-					p.Path, humanize.IBytes(uint64(p.BytesRead)), humanize.IBytes(uint64(p.TotalBytes)), rate)
-			}
-		}
-	}()
+
+	go reportDownloadProgress(progress, done)
 
 	// Track download start time
 	startTime := time.Now()
 
 	// Download each file
-	for _, f := range files {
-		// Sanitize the path to prevent path traversal attacks
-		localPath, err := sanitizePath(f.Path)
-		if err != nil {
-			close(progress)
-			<-done
-			return fmt.Errorf("invalid path in manifest: %w", err)
-		}
-
-		fileURL := baseURL.String() + encodeFilePath(f.Path)
-		log.Infof("fetching %s", f.Path)
-
-		if err := downloadFile(fileURL, localPath, f, progress); err != nil {
-			close(progress)
-			<-done
-			return fmt.Errorf("failed to download %s: %w", f.Path, err)
-		}
-	}
+	dlErr := downloadManifestFiles(ctx.Context, baseURL, files, progress)
 
 	close(progress)
 	<-done
 
+	if dlErr != nil {
+		return dlErr
+	}
+
 	// Print summary
 	elapsed := time.Since(startTime)
 	avgBytesPerSec := float64(totalBytes) / elapsed.Seconds()
-	avgRate := formatBitrate(avgBytesPerSec * 8)
+	avgRate := formatBitrate(avgBytesPerSec * bitsPerByte)
 	log.Infof("downloaded %d files (%s) in %.1fs (%s avg)",
 		len(files),
-		humanize.IBytes(uint64(totalBytes)),
+		humanize.IBytes(safeUint64(totalBytes)),
 		elapsed.Seconds(),
 		avgRate)
 
@@ -145,6 +241,7 @@ func encodeFilePath(p string) string {
 	for i, seg := range segments {
 		segments[i] = url.PathEscape(seg)
 	}
+
 	return strings.Join(segments, "/")
 }
 
@@ -153,12 +250,12 @@ func encodeFilePath(p string) string {
 func sanitizePath(p string) (string, error) {
 	// Reject empty paths
 	if p == "" {
-		return "", fmt.Errorf("empty path")
+		return "", errEmptyPath
 	}
 
 	// Reject absolute paths
 	if filepath.IsAbs(p) {
-		return "", fmt.Errorf("absolute path not allowed: %s", p)
+		return "", fmt.Errorf("%w: %s", errAbsolutePath, p)
 	}
 
 	// Clean the path to resolve . and ..
@@ -166,12 +263,12 @@ func sanitizePath(p string) (string, error) {
 
 	// Reject paths that escape the current directory
 	if strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) || cleaned == ".." {
-		return "", fmt.Errorf("path traversal not allowed: %s", p)
+		return "", fmt.Errorf("%w: %s", errPathTraversal, p)
 	}
 
 	// Also check for absolute paths after cleaning (handles edge cases)
 	if filepath.IsAbs(cleaned) {
-		return "", fmt.Errorf("absolute path not allowed: %s", p)
+		return "", fmt.Errorf("%w: %s", errAbsolutePath, p)
 	}
 
 	return cleaned, nil
@@ -214,10 +311,14 @@ type progressWriter struct {
 
 func (pw *progressWriter) Write(p []byte) (int, error) {
 	n, err := pw.w.Write(p)
+
 	pw.written += int64(n)
 	if pw.progress != nil {
-		var bytesPerSec float64
-		var eta time.Duration
+		var (
+			bytesPerSec float64
+			eta         time.Duration
+		)
+
 		elapsed := time.Since(pw.startTime)
 		if elapsed > 0 && pw.written > 0 {
 			bytesPerSec = float64(pw.written) / elapsed.Seconds()
@@ -226,6 +327,7 @@ func (pw *progressWriter) Write(p []byte) (int, error) {
 				eta = time.Duration(float64(remainingBytes)/bytesPerSec) * time.Second
 			}
 		}
+
 		sendProgress(pw.progress, DownloadProgress{
 			Path:        pw.path,
 			BytesRead:   pw.written,
@@ -234,18 +336,19 @@ func (pw *progressWriter) Write(p []byte) (int, error) {
 			ETA:         eta,
 		})
 	}
+
 	return n, err
 }
 
 // formatBitrate formats a bits-per-second value with appropriate unit prefix.
 func formatBitrate(bps float64) string {
 	switch {
-	case bps >= 1e9:
-		return fmt.Sprintf("%.1f Gbps", bps/1e9)
-	case bps >= 1e6:
-		return fmt.Sprintf("%.1f Mbps", bps/1e6)
-	case bps >= 1e3:
-		return fmt.Sprintf("%.1f Kbps", bps/1e3)
+	case bps >= bpsPerGbps:
+		return fmt.Sprintf("%.1f Gbps", bps/bpsPerGbps)
+	case bps >= bpsPerMbps:
+		return fmt.Sprintf("%.1f Mbps", bps/bpsPerMbps)
+	case bps >= bpsPerKbps:
+		return fmt.Sprintf("%.1f Kbps", bps/bpsPerKbps)
 	default:
 		return fmt.Sprintf("%.0f bps", bps)
 	}
@@ -259,53 +362,100 @@ func sendProgress(ch chan<- DownloadProgress, p DownloadProgress) {
 	}
 }
 
-// downloadFile downloads a URL to a local file path with hash verification.
-// It downloads to a temporary file, verifies the hash, then renames to the final path.
-// Progress is reported via the progress channel.
-func downloadFile(fileURL, localPath string, entry *mfer.MFFilePath, progress chan<- DownloadProgress) error {
-	// Create parent directories if needed
+// tempPathFor computes the temporary download path for a local file.
+// For dotfiles, just append .tmp (they're already hidden); for regular
+// files, prefix with . and append .tmp.
+func tempPathFor(localPath string) string {
 	dir := filepath.Dir(localPath)
-	if dir != "" && dir != "." {
-		if err := os.MkdirAll(dir, 0o755); err != nil {
-			return fmt.Errorf("failed to create directory %s: %w", dir, err)
-		}
-	}
-
-	// Compute temp file path in the same directory
-	// For dotfiles, just append .tmp (they're already hidden)
-	// For regular files, prefix with . and append .tmp
 	base := filepath.Base(localPath)
+
 	var tmpName string
 	if strings.HasPrefix(base, ".") {
 		tmpName = base + ".tmp"
 	} else {
 		tmpName = "." + base + ".tmp"
 	}
-	tmpPath := filepath.Join(dir, tmpName)
+
 	if dir == "" || dir == "." {
-		tmpPath = tmpName
+		return tmpName
 	}
 
+	return filepath.Join(dir, tmpName)
+}
+
+// verifyDownloadedHash checks the computed sha256 digest against the
+// manifest entry's hashes; at least one must match.
+func verifyDownloadedHash(digest []byte, entry *mfer.MFFilePath) error {
+	computed, err := multihash.Encode(digest, multihash.SHA2_256)
+	if err != nil {
+		return fmt.Errorf("failed to encode hash: %w", err)
+	}
+
+	for _, hash := range entry.GetHashes() {
+		if bytes.Equal(computed, hash.GetMultiHash()) {
+			return nil
+		}
+	}
+
+	return errHashMismatch
+}
+
+// downloadFile downloads a URL to a local file path with hash verification.
+// It downloads to a temporary file, verifies the hash, then renames to the final path.
+// Progress is reported via the progress channel.
+func downloadFile(
+	ctx context.Context,
+	fileURL, localPath string,
+	entry *mfer.MFFilePath,
+	progress chan<- DownloadProgress,
+) error {
+	// Enforce the path invariant here rather than relying on the caller,
+	// so every entry point to downloadFile gets the same treatment.
+	localPath, err := sanitizePath(localPath)
+	if err != nil {
+		return fmt.Errorf("invalid path: %w", err)
+	}
+
+	// Create parent directories if needed
+	dir := filepath.Dir(localPath)
+	if dir != "" && dir != "." {
+		err := os.MkdirAll(dir, dirPerms)
+		if err != nil {
+			return fmt.Errorf("failed to create directory %s: %w", dir, err)
+		}
+	}
+
+	tmpPath := tempPathFor(localPath)
+
 	// Fetch file
-	resp, err := http.Get(fileURL) //nolint:gosec // URL constructed from manifest base
+	resp, err := httpGet(ctx, fileURL)
 	if err != nil {
 		return fmt.Errorf("HTTP request failed: %w", err)
 	}
+
 	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("HTTP %d", resp.StatusCode)
+		return fmt.Errorf("%w %d", errHTTPStatus, resp.StatusCode)
 	}
 
 	// Determine expected size
-	expectedSize := entry.Size
+	expectedSize := entry.GetSize()
+
 	totalBytes := resp.ContentLength
 	if totalBytes < 0 {
 		totalBytes = expectedSize
 	}
 
-	// Create temp file
-	out, err := os.Create(tmpPath)
+	// Create temp file.
+	//
+	// G304: tmpPath is derived from localPath, which sanitizePath above
+	// constrains lexically to a relative path that does not escape the
+	// destination directory. That is a purely lexical guarantee: it does
+	// not resolve symlinks, so a pre-existing symlink inside the
+	// destination tree can still redirect this write outside of it
+	// (tracked in issue #86).
+	out, err := os.Create(tmpPath) //nolint:gosec // G304: see comment above
 	if err != nil {
 		return fmt.Errorf("failed to create temp file: %w", err)
 	}
@@ -328,45 +478,50 @@ func downloadFile(fileURL, localPath string, entry *mfer.MFFilePath, progress ch
 	// Close file before checking errors (to flush writes)
 	closeErr := out.Close()
 
-	// If copy failed, clean up temp file and return error
-	if copyErr != nil {
+	err = finishDownload(
+		tmpPath, localPath, written, expectedSize, h.Sum(nil), entry,
+		copyErr, closeErr)
+	if err != nil {
 		_ = os.Remove(tmpPath)
+
+		return err
+	}
+
+	return nil
+}
+
+// finishDownload validates the copy result, verifies size and hash, and
+// moves the temp file into place. On error the caller removes tmpPath.
+func finishDownload(
+	tmpPath, localPath string,
+	written, expectedSize int64,
+	digest []byte,
+	entry *mfer.MFFilePath,
+	copyErr, closeErr error,
+) error {
+	if copyErr != nil {
 		return copyErr
 	}
+
 	if closeErr != nil {
-		_ = os.Remove(tmpPath)
 		return closeErr
 	}
 
 	// Verify size
 	if written != expectedSize {
-		_ = os.Remove(tmpPath)
-		return fmt.Errorf("size mismatch: expected %d bytes, got %d", expectedSize, written)
-	}
-
-	// Encode computed hash as multihash
-	computed, err := multihash.Encode(h.Sum(nil), multihash.SHA2_256)
-	if err != nil {
-		_ = os.Remove(tmpPath)
-		return fmt.Errorf("failed to encode hash: %w", err)
+		return fmt.Errorf("%w: expected %d bytes, got %d",
+			errSizeMismatch, expectedSize, written)
 	}
 
 	// Verify hash against manifest (at least one must match)
-	hashMatch := false
-	for _, hash := range entry.Hashes {
-		if bytes.Equal(computed, hash.MultiHash) {
-			hashMatch = true
-			break
-		}
-	}
-	if !hashMatch {
-		_ = os.Remove(tmpPath)
-		return fmt.Errorf("hash mismatch")
+	err := verifyDownloadedHash(digest, entry)
+	if err != nil {
+		return err
 	}
 
 	// Rename temp file to final path
-	if err := os.Rename(tmpPath, localPath); err != nil {
-		_ = os.Remove(tmpPath)
+	err = os.Rename(tmpPath, localPath)
+	if err != nil {
 		return fmt.Errorf("failed to rename temp file: %w", err)
 	}