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

Updates golangci-lint to v2.12.2 everywhere it is pinned and installs the canonical `.golangci.yml`, then remediates every finding the new linter/config surfaces so `make check` is green. ## Version bump - `Dockerfile` lint stage: `golangci/golangci-lint:v2.11.3-alpine` -> `v2.12.2-alpine` (digest-pinned, date comment updated) - `Makefile` `deps` target: `go install` moved from the old v1 module path at `@latest` to the pinned `github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2` - `.golangci.yml` replaced with the canonical config (v2 schema; settings under `linters.settings` so the thresholds actually apply; `default: all` with the standard six disables) - `script/bootstrap` installs golangci-lint via the system package manager and carries no version pin, so it is unchanged - CI (`.gitea/workflows/check.yml`) only runs `script/cibuild`, so it needed no change ## Lint remediation The canonical config surfaced ~3,300 findings across 56k lines. All are fixed, behavior-preserving; incorporates and supersedes the per-package mechanical passes already merged to `main` (refs #61). Highlights: - `err113`: dynamic errors replaced with package sentinels + `%w` wrapping; comparisons via `errors.Is` - `goprintffuncname`: printf-style helpers renamed with an `f` suffix (`ui.Writer` message methods, `cli.ReportErrorf`, `database.Fatalf`) and all call sites updated - `revive` stutter renames: `blob.Handler`, `blob.WithReader`, `blob.ChunkPosition`, `storage.URL`, `storage.Info`; missing doc comments added - `contextcheck`/`noctx`: `context.Context` threaded through `blob.Packer` and the scanner call sites; context-aware `exec`/`sql` variants - `funlen`/`cyclop`/`gocognit`/`dupl`: oversized and duplicated functions split into focused helpers (production and test code) - tests: `t.Parallel()` added where safe (global logger init kept in the serial phase for `-race`), `t.TempDir()`/`t.Helper()` adopted, several suites converted to external test packages - `gosec`: bounded integer conversions, `ReadHeaderTimeout` on the test HTTP server; remaining warnings suppressed per-site with justifications - remaining `nolint` directives are rare, targeted, and each carries a reason (e.g. `nilnil` not-found contract in the repository layer, fx module globals, on-disk snake_case struct tags) - removed the deprecated `log.LogOptions` alias (callers migrated to `log.Options`) `make check` (tests with `-race`, lint, fmt-check) passes. Co-authored-by: sneak <sneak@sneak.berlin> Reviewed-on: #62 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-08-07 23:22:48 +02:00
parent b87b72d4b9
commit cc58583130
126 changed files with 8184 additions and 5470 deletions
--- a/internal/storage/file.go
+++ b/internal/storage/file.go
@@ -42,18 +42,19 @@ func (f *FileStorer) SetFilesystem(fs afero.Fs) {
 	f.fs = fs
 }

-// fullPath returns the full filesystem path for a key.
-func (f *FileStorer) fullPath(key string) string {
-	return filepath.Join(f.basePath, key)
-}
+// storageDirPerm is the mode used for directories created under the
+// storage base path.
+const storageDirPerm = 0o755

 // Put stores data at the specified key.
-func (f *FileStorer) Put(ctx context.Context, key string, data io.Reader) error {
+func (f *FileStorer) Put(_ context.Context, key string, data io.Reader) error {
 	path := f.fullPath(key)

 	// Create parent directories
 	dir := filepath.Dir(path)
-	if err := f.fs.MkdirAll(dir, 0755); err != nil {
+
+	err := f.fs.MkdirAll(dir, storageDirPerm)
+	if err != nil {
 		return fmt.Errorf("creating directories: %w", err)
 	}

@@ -72,12 +73,17 @@ func (f *FileStorer) Put(ctx context.Context, key string, data io.Reader) error
 }

 // PutWithProgress stores data with progress reporting.
-func (f *FileStorer) PutWithProgress(ctx context.Context, key string, data io.Reader, size int64, progress ProgressCallback) error {
+func (f *FileStorer) PutWithProgress(
+	_ context.Context, key string, data io.Reader,
+	_ int64, progress ProgressCallback,
+) error {
 	path := f.fullPath(key)

 	// Create parent directories
 	dir := filepath.Dir(path)
-	if err := f.fs.MkdirAll(dir, 0755); err != nil {
+
+	err := f.fs.MkdirAll(dir, storageDirPerm)
+	if err != nil {
 		return fmt.Errorf("creating directories: %w", err)
 	}

@@ -102,7 +108,7 @@ func (f *FileStorer) PutWithProgress(ctx context.Context, key string, data io.Re
 }

 // Get retrieves data from the specified key.
-func (f *FileStorer) Get(ctx context.Context, key string) (io.ReadCloser, error) {
+func (f *FileStorer) Get(_ context.Context, key string) (io.ReadCloser, error) {
 	path := f.fullPath(key)

 	file, err := f.fs.Open(path)
@@ -118,7 +124,7 @@ func (f *FileStorer) Get(ctx context.Context, key string) (io.ReadCloser, error)
 }

 // Stat returns metadata about an object without retrieving its contents.
-func (f *FileStorer) Stat(ctx context.Context, key string) (*ObjectInfo, error) {
+func (f *FileStorer) Stat(_ context.Context, key string) (*ObjectInfo, error) {
 	path := f.fullPath(key)

 	info, err := f.fs.Stat(path)
@@ -137,7 +143,7 @@ func (f *FileStorer) Stat(ctx context.Context, key string) (*ObjectInfo, error)
 }

 // Delete removes an object.
-func (f *FileStorer) Delete(ctx context.Context, key string) error {
+func (f *FileStorer) Delete(_ context.Context, key string) error {
 	path := f.fullPath(key)

 	err := f.fs.Remove(path)
@@ -233,7 +239,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
 			if err != nil {
 				ch <- ObjectInfo{Err: err}

-				return nil // Continue walking despite errors
+				return nil //nolint:nilerr // continue walking despite errors
 			}

 			if !info.IsDir() {
@@ -259,13 +265,18 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
 }

 // Info returns human-readable storage location information.
-func (f *FileStorer) Info() StorageInfo {
-	return StorageInfo{
-		Type:     "file",
+func (f *FileStorer) Info() Info {
+	return Info{
+		Type:     schemeFile,
 		Location: f.basePath,
 	}
 }

+// fullPath returns the full filesystem path for a key.
+func (f *FileStorer) fullPath(key string) string {
+	return filepath.Join(f.basePath, key)
+}
+
 // progressWriter wraps an io.Writer to track write progress.
 type progressWriter struct {
 	writer   io.Writer
--- a/internal/storage/module.go
+++ b/internal/storage/module.go
@@ -10,9 +10,17 @@ import (
 	"sneak.berlin/go/vaultik/internal/s3"
 )

+// defaultS3Region is used when neither the URL nor the config specify one.
+const defaultS3Region = "us-east-1"
+
+// defaultS3Endpoint is the AWS endpoint used when none is configured.
+const defaultS3Endpoint = "s3.amazonaws.com"
+
 // Module exports storage functionality as an fx module.
 // It provides a Storer implementation based on the configured storage URL
 // or falls back to legacy S3 configuration.
+//
+//nolint:gochecknoglobals // fx module definitions are package globals
 var Module = fx.Module("storage",
 	fx.Provide(NewStorer),
 )
@@ -20,6 +28,8 @@ var Module = fx.Module("storage",
 // NewStorer creates a Storer based on configuration.
 // If StorageURL is set, it uses URL-based configuration.
 // Otherwise, it falls back to legacy S3 configuration.
+//
+//nolint:ireturn // fx provider intentionally returns the Storer interface
 func NewStorer(cfg *config.Config) (Storer, error) {
 	if cfg.StorageURL != "" {
 		return storerFromURL(cfg.StorageURL, cfg)
@@ -28,6 +38,7 @@ func NewStorer(cfg *config.Config) (Storer, error) {
 	return storerFromLegacyS3Config(cfg)
 }

+//nolint:ireturn // factory intentionally returns the Storer interface
 func storerFromURL(rawURL string, cfg *config.Config) (Storer, error) {
 	parsed, err := ParseStorageURL(rawURL)
 	if err != nil {
@@ -35,59 +46,74 @@ func storerFromURL(rawURL string, cfg *config.Config) (Storer, error) {
 	}

 	switch parsed.Scheme {
-	case "file":
+	case schemeFile:
 		return NewFileStorer(parsed.Prefix)

-	case "s3":
-		// Build endpoint URL
-		endpoint := parsed.Endpoint
-		if endpoint == "" {
-			endpoint = "s3.amazonaws.com"
-		}
+	case schemeS3:
+		return storerFromParsedS3URL(parsed, cfg)

-		// Add protocol if not present
-		if parsed.UseSSL && !strings.HasPrefix(endpoint, "https://") && !strings.HasPrefix(endpoint, "http://") {
-			endpoint = "https://" + endpoint
-		} else if !parsed.UseSSL && !strings.HasPrefix(endpoint, "http://") && !strings.HasPrefix(endpoint, "https://") {
-			endpoint = "http://" + endpoint
-		}
-
-		region := parsed.Region
-		if region == "" {
-			region = cfg.S3.Region
-			if region == "" {
-				region = "us-east-1"
-			}
-		}
-
-		// Credentials come from config (not URL for security)
-		client, err := s3.NewClient(context.Background(), s3.Config{
-			Endpoint:        endpoint,
-			Bucket:          parsed.Bucket,
-			Prefix:          parsed.Prefix,
-			AccessKeyID:     cfg.S3.AccessKeyID,
-			SecretAccessKey: cfg.S3.SecretAccessKey,
-			Region:          region,
-		})
-		if err != nil {
-			return nil, fmt.Errorf("creating S3 client: %w", err)
-		}
-
-		return NewS3Storer(client), nil
-
-	case "rclone":
-		return NewRcloneStorer(context.Background(), parsed.RcloneRemote, parsed.Prefix)
+	case schemeRclone:
+		return NewRcloneStorer(
+			context.Background(), parsed.RcloneRemote, parsed.Prefix)

 	default:
-		return nil, fmt.Errorf("unsupported storage scheme: %s", parsed.Scheme)
+		return nil, fmt.Errorf("%w: %s", ErrUnsupportedStorage, parsed.Scheme)
 	}
 }

+// storerFromParsedS3URL builds an S3 storer from a parsed s3:// URL,
+// filling endpoint protocol and region defaults from the config.
+//
+//nolint:ireturn // factory intentionally returns the Storer interface
+func storerFromParsedS3URL(parsed *URL, cfg *config.Config) (Storer, error) {
+	// Build endpoint URL
+	endpoint := parsed.Endpoint
+	if endpoint == "" {
+		endpoint = defaultS3Endpoint
+	}
+
+	// Add protocol if not present
+	hasProtocol := strings.HasPrefix(endpoint, "https://") ||
+		strings.HasPrefix(endpoint, "http://")
+	if !hasProtocol {
+		if parsed.UseSSL {
+			endpoint = "https://" + endpoint
+		} else {
+			endpoint = "http://" + endpoint
+		}
+	}
+
+	region := parsed.Region
+	if region == "" {
+		region = cfg.S3.Region
+		if region == "" {
+			region = defaultS3Region
+		}
+	}
+
+	// Credentials come from config (not URL for security)
+	client, err := s3.NewClient(context.Background(), s3.Config{
+		Endpoint:        endpoint,
+		Bucket:          parsed.Bucket,
+		Prefix:          parsed.Prefix,
+		AccessKeyID:     cfg.S3.AccessKeyID,
+		SecretAccessKey: cfg.S3.SecretAccessKey,
+		Region:          region,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("creating S3 client: %w", err)
+	}
+
+	return NewS3Storer(client), nil
+}
+
+//nolint:ireturn // factory intentionally returns the Storer interface
 func storerFromLegacyS3Config(cfg *config.Config) (Storer, error) {
 	endpoint := cfg.S3.Endpoint

 	// Ensure protocol is present
-	if !strings.HasPrefix(endpoint, "http://") && !strings.HasPrefix(endpoint, "https://") {
+	if !strings.HasPrefix(endpoint, "http://") &&
+		!strings.HasPrefix(endpoint, "https://") {
 		if cfg.S3.UseSSL {
 			endpoint = "https://" + endpoint
 		} else {
@@ -97,7 +123,7 @@ func storerFromLegacyS3Config(cfg *config.Config) (Storer, error) {

 	region := cfg.S3.Region
 	if region == "" {
-		region = "us-east-1"
+		region = defaultS3Region
 	}

 	client, err := s3.NewClient(context.Background(), s3.Config{
--- a/internal/storage/rclone.go
+++ b/internal/storage/rclone.go
@@ -69,7 +69,8 @@ func (r *RcloneStorer) Put(ctx context.Context, key string, data io.Reader) erro
 	}

 	// Upload the object
-	_, err = operations.Rcat(ctx, r.fsys, key, io.NopCloser(bytes.NewReader(buf)), time.Now(), nil)
+	_, err = operations.Rcat(ctx, r.fsys, key,
+		io.NopCloser(bytes.NewReader(buf)), time.Now(), nil)
 	if err != nil {
 		return fmt.Errorf("uploading object: %w", err)
 	}
@@ -78,7 +79,10 @@ func (r *RcloneStorer) Put(ctx context.Context, key string, data io.Reader) erro
 }

 // PutWithProgress stores data with progress reporting.
-func (r *RcloneStorer) PutWithProgress(ctx context.Context, key string, data io.Reader, size int64, progress ProgressCallback) error {
+func (r *RcloneStorer) PutWithProgress(
+	ctx context.Context, key string, data io.Reader,
+	_ int64, progress ProgressCallback,
+) error {
 	// Wrap reader with progress tracking
 	pr := &progressReader{
 		reader:   data,
@@ -181,7 +185,9 @@ func (r *RcloneStorer) List(ctx context.Context, prefix string) ([]string, error
 }

 // ListStream returns a channel of ObjectInfo for large result sets.
-func (r *RcloneStorer) ListStream(ctx context.Context, prefix string) <-chan ObjectInfo {
+func (r *RcloneStorer) ListStream(
+	ctx context.Context, prefix string,
+) <-chan ObjectInfo {
 	ch := make(chan ObjectInfo)

 	go func() {
@@ -212,14 +218,14 @@ func (r *RcloneStorer) ListStream(ctx context.Context, prefix string) <-chan Obj
 }

 // Info returns human-readable storage location information.
-func (r *RcloneStorer) Info() StorageInfo {
+func (r *RcloneStorer) Info() Info {
 	location := r.remote
 	if r.path != "" {
 		location += ":" + r.path
 	}

-	return StorageInfo{
-		Type:     "rclone",
+	return Info{
+		Type:     schemeRclone,
 		Location: location,
 	}
 }
--- a/internal/storage/s3.go
+++ b/internal/storage/s3.go
@@ -24,7 +24,10 @@ func (s *S3Storer) Put(ctx context.Context, key string, data io.Reader) error {
 }

 // PutWithProgress stores data with progress reporting.
-func (s *S3Storer) PutWithProgress(ctx context.Context, key string, data io.Reader, size int64, progress ProgressCallback) error {
+func (s *S3Storer) PutWithProgress(
+	ctx context.Context, key string, data io.Reader,
+	size int64, progress ProgressCallback,
+) error {
 	// Convert storage.ProgressCallback to s3.ProgressCallback
 	var s3Progress s3.ProgressCallback
 	if progress != nil {
@@ -81,8 +84,8 @@ func (s *S3Storer) ListStream(ctx context.Context, prefix string) <-chan ObjectI
 }

 // Info returns human-readable storage location information.
-func (s *S3Storer) Info() StorageInfo {
-	return StorageInfo{
+func (s *S3Storer) Info() Info {
+	return Info{
 		Type:     "s3",
 		Location: fmt.Sprintf("%s/%s", s.client.Endpoint(), s.client.BucketName()),
 	}
--- a/internal/storage/storer.go
+++ b/internal/storage/storer.go
@@ -30,14 +30,15 @@ type ObjectInfo struct {
 	Err  error  // Error for streaming results (nil on success)
 }

-// StorageInfo provides human-readable storage configuration.
-type StorageInfo struct {
+// Info provides human-readable storage configuration.
+type Info struct {
 	Type     string // "s3" or "file"
 	Location string // endpoint/bucket for S3, base path for filesystem
 }

 // Storer defines the interface for storage backends.
-// All paths are relative to the storage root (bucket/prefix for S3, base directory for filesystem).
+// All paths are relative to the storage root (bucket/prefix for S3, base
+// directory for filesystem).
 type Storer interface {
 	// Put stores data at the specified key.
 	// Parent directories are created automatically for filesystem backends.
@@ -46,7 +47,8 @@ type Storer interface {
 	// PutWithProgress stores data with progress reporting.
 	// Size must be the exact size of the data to store.
 	// The progress callback is called periodically with bytes transferred.
-	PutWithProgress(ctx context.Context, key string, data io.Reader, size int64, progress ProgressCallback) error
+	PutWithProgress(ctx context.Context, key string, data io.Reader,
+		size int64, progress ProgressCallback) error

 	// Get retrieves data from the specified key.
 	// The caller must close the returned ReadCloser.
@@ -70,5 +72,5 @@ type Storer interface {
 	ListStream(ctx context.Context, prefix string) <-chan ObjectInfo

 	// Info returns human-readable storage location information.
-	Info() StorageInfo
+	Info() Info
 }
--- a/internal/storage/url.go
+++ b/internal/storage/url.go
@@ -7,8 +7,26 @@ import (
 	"strings"
 )

-// StorageURL represents a parsed storage URL.
-type StorageURL struct {
+// Storage URL scheme names.
+const (
+	schemeFile   = "file"
+	schemeS3     = "s3"
+	schemeRclone = "rclone"
+)
+
+// Sentinel errors for storage URL parsing.
+var (
+	ErrEmptyStorageURL   = errors.New("storage URL is empty")
+	ErrEmptyFilePath     = errors.New("file URL path is empty")
+	ErrMissingBucket     = errors.New("s3 URL missing bucket name")
+	ErrMissingRemote     = errors.New("rclone URL missing remote name")
+	ErrUnsupportedScheme = errors.New(
+		"unsupported URL scheme: must start with s3://, file://, or rclone://")
+	ErrUnsupportedStorage = errors.New("unsupported storage scheme")
+)
+
+// URL represents a parsed storage URL.
+type URL struct {
 	Scheme       string // "s3", "file", or "rclone"
 	Bucket       string // S3 bucket name (empty for file/rclone)
 	Prefix       string // Path within bucket or filesystem base path
@@ -23,20 +41,20 @@ type StorageURL struct {
 //   - s3://bucket/prefix?endpoint=host&region=us-east-1&ssl=true
 //   - file:///absolute/path/to/backup
 //   - rclone://remote/path/to/backups
-func ParseStorageURL(rawURL string) (*StorageURL, error) {
+func ParseStorageURL(rawURL string) (*URL, error) {
 	if rawURL == "" {
-		return nil, errors.New("storage URL is empty")
+		return nil, ErrEmptyStorageURL
 	}

 	// Handle file:// URLs
 	if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
 		path := after
 		if path == "" {
-			return nil, errors.New("file URL path is empty")
+			return nil, ErrEmptyFilePath
 		}

-		return &StorageURL{
-			Scheme: "file",
+		return &URL{
+			Scheme: schemeFile,
 			Prefix: path,
 		}, nil
 	}
@@ -50,7 +68,7 @@ func ParseStorageURL(rawURL string) (*StorageURL, error) {

 		bucket := u.Host
 		if bucket == "" {
-			return nil, errors.New("s3 URL missing bucket name")
+			return nil, ErrMissingBucket
 		}

 		prefix := strings.TrimPrefix(u.Path, "/")
@@ -62,8 +80,8 @@ func ParseStorageURL(rawURL string) (*StorageURL, error) {
 			useSSL = false
 		}

-		return &StorageURL{
-			Scheme:   "s3",
+		return &URL{
+			Scheme:   schemeS3,
 			Bucket:   bucket,
 			Prefix:   prefix,
 			Endpoint: query.Get("endpoint"),
@@ -81,27 +99,27 @@ func ParseStorageURL(rawURL string) (*StorageURL, error) {

 		remote := u.Host
 		if remote == "" {
-			return nil, errors.New("rclone URL missing remote name")
+			return nil, ErrMissingRemote
 		}

 		path := strings.TrimPrefix(u.Path, "/")

-		return &StorageURL{
-			Scheme:       "rclone",
+		return &URL{
+			Scheme:       schemeRclone,
 			Prefix:       path,
 			RcloneRemote: remote,
 		}, nil
 	}

-	return nil, errors.New("unsupported URL scheme: must start with s3://, file://, or rclone://")
+	return nil, ErrUnsupportedScheme
 }

 // String returns a human-readable representation of the storage URL.
-func (u *StorageURL) String() string {
+func (u *URL) String() string {
 	switch u.Scheme {
-	case "file":
+	case schemeFile:
 		return "file://" + u.Prefix
-	case "s3":
+	case schemeS3:
 		endpoint := u.Endpoint
 		if endpoint == "" {
 			endpoint = "s3.amazonaws.com"
@@ -112,7 +130,7 @@ func (u *StorageURL) String() string {
 		}

 		return fmt.Sprintf("s3://%s (endpoint: %s)", u.Bucket, endpoint)
-	case "rclone":
+	case schemeRclone:
 		if u.Prefix != "" {
 			return fmt.Sprintf("rclone://%s/%s", u.RcloneRemote, u.Prefix)
 		}