Merge pull request #429 from sapcc/fix-lints

fix new lints

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/prometheus/client_golang v1.20.3
 	github.com/redis/go-redis/v9 v9.6.1
 	github.com/rs/cors v1.11.1
-	github.com/sapcc/go-api-declarations v1.12.5
+	github.com/sapcc/go-api-declarations v1.12.6
 	github.com/sapcc/go-bits v0.0.0-20240905070742-0ac071bc79e7
 	github.com/spf13/cobra v1.8.1
 	github.com/timewasted/go-accept-headers v0.0.0-20130320203746-c78f304b1b09

go.sum

@@ -187,8 +187,8 @@ github.com/rs/cors v1.11.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/samber/lo v1.46.0 h1:w8G+oaCPgz1PoCJztqymCFaKwXt+5cCXn51uPxExFfQ=
 github.com/samber/lo v1.46.0/go.mod h1:RmDH9Ct32Qy3gduHQuKJ3gW1fMHAnE/fAzQuf6He5cU=
-github.com/sapcc/go-api-declarations v1.12.5 h1:NjA3ydh9VyxSGGxYEaufmK9Jqk2WC9OM/vi1mktkIu8=
-github.com/sapcc/go-api-declarations v1.12.5/go.mod h1:83R3hTANhuRXt/pXDby37IJetw8l7DG41s33Tp9NXxI=
+github.com/sapcc/go-api-declarations v1.12.6 h1:04sQmS3ipzPc+ENSCWW9A4FgCN7Qa5JIbNUNh7ziQJA=
+github.com/sapcc/go-api-declarations v1.12.6/go.mod h1:83R3hTANhuRXt/pXDby37IJetw8l7DG41s33Tp9NXxI=
 github.com/sapcc/go-bits v0.0.0-20240905070742-0ac071bc79e7 h1:Y5PXn4j/KA6T4sse0+kbvDq5KzVcp8X8BAcfjeauwBQ=
 github.com/sapcc/go-bits v0.0.0-20240905070742-0ac071bc79e7/go.mod h1:DaDx6wsOyTlpmPIxklfOD6SgR0Au05FQenghLb/ALic=
 github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN3Uc8sB6B/s6Z4t2xvBgU1htSHuq8=

internal/api/keppel/manifests_test.go

@@ -117,7 +117,7 @@ func TestManifestsAPI(t *testing.T) {
 					RepositoryID:      int64(repoID),
 					Digest:            dummyDigest,
 					MediaType:         schema2.MediaTypeManifest,
-					SizeBytes:         uint64(sizeBytes),
+					SizeBytes:         uint64(sizeBytes), //nolint:gosec // construction guarantees that value is positive
 					PushedAt:          pushedAt,
 					NextValidationAt:  pushedAt.Add(models.ManifestValidationInterval),
 					LabelsJSON:        `{"foo":"is there"}`,
@@ -176,7 +176,7 @@ func TestManifestsAPI(t *testing.T) {
 			renderedManifests[idx-1] = assert.JSONObject{
 				"digest":               test.DeterministicDummyDigest(10 + idx),
 				"media_type":           schema2.MediaTypeManifest,
-				"size_bytes":           uint64(1000 * idx),
+				"size_bytes":           uint64(1000 * idx), //nolint:gosec // construction guarantees that value is positive
 				"pushed_at":            int64(1000 * (10 + idx)),
 				"last_pulled_at":       nil,
 				"labels":               assert.JSONObject{"foo": "is there"},

internal/api/keppel/quotas_test.go

@@ -48,7 +48,6 @@ func TestQuotasAPI(t *testing.T) {
 	buildLiquidResponse := func(quota, usage uint64) assert.JSONObject {
 		return assert.JSONObject{
 			"infoVersion": 1,
-			"metrics":     map[string]assert.JSONObject{},
 			"resources": map[string]assert.JSONObject{
 				"images": {
 					"forbidden": false,
@@ -226,7 +225,7 @@ func TestQuotasAPI(t *testing.T) {
 			RepositoryID:     1,
 			Digest:           test.DeterministicDummyDigest(idx),
 			MediaType:        "",
-			SizeBytes:        uint64(1000 * idx),
+			SizeBytes:        uint64(1000 * idx), //nolint:gosec // construction guarantees that value is positive
 			PushedAt:         pushedAt,
 			NextValidationAt: pushedAt.Add(models.ManifestValidationInterval),
 		})

internal/api/keppel/repos_test.go

@@ -106,7 +106,7 @@ func TestReposAPI(t *testing.T) {
 		blob := models.Blob{
 			AccountName:      "test1",
 			Digest:           dummyDigest,
-			SizeBytes:        uint64(2000 * idx),
+			SizeBytes:        uint64(2000 * idx), //nolint:gosec // construction guarantees that value is positive
 			PushedAt:         blobPushedAt,
 			NextValidationAt: blobPushedAt.Add(models.BlobValidationInterval),
 		}
@@ -126,7 +126,7 @@ func TestReposAPI(t *testing.T) {
 			RepositoryID:     filledRepo.ID,
 			Digest:           dummyDigest,
 			MediaType:        "",
-			SizeBytes:        uint64(1000 * idx),
+			SizeBytes:        uint64(1000 * idx), //nolint:gosec // construction guarantees that value is positive
 			PushedAt:         manifestPushedAt,
 			NextValidationAt: manifestPushedAt.Add(models.ManifestValidationInterval),
 		})

internal/api/registry/tags_test.go

@@ -96,7 +96,7 @@ func TestListTags(t *testing.T) {
 		}.Check(t, h)
 
 		// test paginated
-		for offset := range len(allTagNames) {
+		for offset := range allTagNames {
 			for length := 1; length <= len(allTagNames)+1; length++ {
 				expectedPage := allTagNames[offset:]
 				expectedHeaders := map[string]string{

internal/api/utils.go

@@ -49,7 +49,7 @@ func CheckRateLimit(r *http.Request, rle *keppel.RateLimitEngine, account models
 		return err
 	}
 	if !allowed {
-		retryAfterStr := strconv.FormatUint(uint64(result.RetryAfter/time.Second), 10)
+		retryAfterStr := strconv.FormatUint(keppel.AtLeastZero(int64(result.RetryAfter/time.Second)), 10)
 		return keppel.ErrTooManyRequests.With("").WithHeader("Retry-After", retryAfterStr)
 	}
 

internal/drivers/filesystem/storage.go

@@ -115,7 +115,7 @@ func (d *StorageDriver) ReadBlob(ctx context.Context, account models.Account, st
 		f.Close()
 		return nil, 0, err
 	}
-	return f, uint64(stat.Size()), nil
+	return f, keppel.AtLeastZero(stat.Size()), nil
 }
 
 // URLForBlob implements the keppel.StorageDriver interface.

internal/drivers/openstack/swift.go

@@ -378,7 +378,7 @@ func (d *swiftDriver) ListStorageContents(ctx context.Context, account models.Ac
 			if err != nil {
 				return fmt.Errorf("while parsing chunk object name %s: %s", o.Name(), err.Error())
 			}
-			mergeChunkCount(chunkCounts, storageID, uint32(chunkNumber)) //nolint:gosec // we use ParseUint with 32 bits above
+			mergeChunkCount(chunkCounts, storageID, uint32(chunkNumber))
 			return nil
 		}
 		if match := manifestObjectNameRx.FindStringSubmatch(o.Name()); match != nil {

internal/keppel/database_helpers.go

@@ -154,7 +154,16 @@ var manifestUsageQuery = sqlext.SimplifyWhitespace(`
 // accounts connected to this quota set's auth tenant.
 func GetManifestUsage(db gorp.SqlExecutor, quotas models.Quotas) (uint64, error) {
 	manifestCount, err := db.SelectInt(manifestUsageQuery, quotas.AuthTenantID)
-	return uint64(manifestCount), err
+	return AtLeastZero(manifestCount), err
+}
+
+// AtLeastZero safely converts int or int64 values (which might come from
+// DB.SelectInt() or from IO reads/writes) to uint64 by clamping negative values to 0.
+func AtLeastZero[I interface{ int | int64 }](x I) uint64 {
+	if x < 0 {
+		return 0
+	}
+	return uint64(x)
 }
 
 // FindOrCreateRepository works similar to db.SelectOne(), but autovivifies a

internal/keppel/ratelimit_driver.go

@@ -124,7 +124,7 @@ func (e RateLimitEngine) RateLimitAllows(ctx context.Context, remoteAddr string,
 
 	limiter := redis_rate.NewLimiter(e.Client)
 	key := fmt.Sprintf("keppel-ratelimit-%s-%s-%s", remoteAddr, account.Name, string(action))
-	result, err := limiter.AllowN(ctx, key, *rateQuota, int(amount)) //nolint:gosec // we check above if the value is above math.MaxInt
+	result, err := limiter.AllowN(ctx, key, *rateQuota, int(amount))
 	if err != nil {
 		return false, &redis_rate.Result{}, err
 	}

internal/processor/accounts.go

@@ -393,7 +393,7 @@ func (p *Processor) DeleteAccount(ctx context.Context, account models.Account, a
 		manifestCount, err := p.db.SelectInt(deleteAccountCountManifestsQuery, account.Name)
 		return &DeleteAccountResponse{
 			RemainingManifests: &DeleteAccountRemainingManifests{
-				Count: uint64(manifestCount),
+				Count: keppel.AtLeastZero(manifestCount),
 				Next:  nextManifests,
 			},
 		}, err
@@ -422,7 +422,7 @@ func (p *Processor) DeleteAccount(ctx context.Context, account models.Account, a
 			return nil, err
 		}
 		return &DeleteAccountResponse{
-			RemainingBlobs: &DeleteAccountRemainingBlobs{Count: uint64(blobCount)},
+			RemainingBlobs: &DeleteAccountRemainingBlobs{Count: keppel.AtLeastZero(blobCount)},
 		}, nil
 	}
 

internal/processor/blobs.go

@@ -73,7 +73,7 @@ func (p *Processor) ValidateExistingBlob(ctx context.Context, account models.Acc
 		)
 	}
 
-	if uint64(bcw.bytesWritten) != blob.SizeBytes {
+	if bcw.bytesWritten != blob.SizeBytes {
 		return fmt.Errorf("expected %d bytes, but got %d bytes",
 			blob.SizeBytes, bcw.bytesWritten,
 		)
@@ -84,11 +84,11 @@ func (p *Processor) ValidateExistingBlob(ctx context.Context, account models.Acc
 
 // An io.Writer that just counts how many bytes were written into it.
 type byteCountingWriter struct {
-	bytesWritten int
+	bytesWritten uint64
 }
 
 func (w *byteCountingWriter) Write(buf []byte) (int, error) {
-	w.bytesWritten += len(buf)
+	w.bytesWritten += uint64(len(buf))
 	return len(buf), nil
 }
 
@@ -109,7 +109,7 @@ func (p *Processor) FindBlobOrInsertUnbackedBlob(ctx context.Context, desc distr
 			AccountName:      account.Name,
 			Digest:           desc.Digest,
 			MediaType:        desc.MediaType,
-			SizeBytes:        uint64(desc.Size),
+			SizeBytes:        keppel.AtLeastZero(desc.Size),
 			StorageID:        "", // unbacked
 			PushedAt:         time.Unix(0, 0),
 			NextValidationAt: time.Unix(0, 0),
@@ -340,7 +340,7 @@ func (r *chunkingTrackingReader) Read(buf []byte) (int, error) {
 	}
 
 	n, err := r.wrapped.Read(buf)
-	r.bytesRead += uint64(n)
+	r.bytesRead += keppel.AtLeastZero(n)
 	return n, err
 }
 

internal/processor/manifests.go

@@ -201,9 +201,9 @@ func (p *Processor) validateAndStoreManifestCommon(ctx context.Context, account
 	manifest.MediaType = manifestDesc.MediaType
 	// ^ Those two should be the same already, but if in doubt, we trust the
 	// parser more than the user input.
-	manifest.SizeBytes = uint64(manifestDesc.Size)
+	manifest.SizeBytes = keppel.AtLeastZero(manifestDesc.Size)
 	for _, desc := range manifestParsed.BlobReferences() {
-		manifest.SizeBytes += uint64(desc.Size)
+		manifest.SizeBytes += keppel.AtLeastZero(desc.Size)
 	}
 
 	return p.insideTransaction(ctx, func(ctx context.Context, tx *gorp.Transaction) error {
@@ -316,7 +316,7 @@ func findManifestReferencedObjects(tx *gorp.Transaction, account models.Account,
 		}
 
 		// check that the blob size matches what the manifest says
-		if blob.SizeBytes != uint64(desc.Size) {
+		if blob.SizeBytes != keppel.AtLeastZero(desc.Size) {
 			msg := fmt.Sprintf(
 				"manifest references blob %s with %d bytes, but blob actually contains %d bytes",
 				desc.Digest, desc.Size, blob.SizeBytes)

internal/test/content.go

@@ -231,11 +231,11 @@ func GenerateImageWithCustomConfig(change func(map[string]any), layers ...Bytes)
 // SizeBytes returns the value that we expect in the DB column
 // `manifests.size_bytes` for this image.
 func (i Image) SizeBytes() uint64 {
-	imageSize := len(i.Manifest.Contents) + len(i.Config.Contents)
+	imageSize := uint64(len(i.Manifest.Contents)) + uint64(len(i.Config.Contents))
 	for _, layer := range i.Layers {
-		imageSize += len(layer.Contents)
+		imageSize += uint64(len(layer.Contents))
 	}
-	return uint64(imageSize)
+	return imageSize
 }
 
 // DigestRef returns the ManifestReference for this manifest's digest.
@@ -297,11 +297,11 @@ func GenerateImageList(images ...Image) ImageList {
 // SizeBytes returns the value that we expect in the DB column
 // `manifests.size_bytes` for this image.
 func (l ImageList) SizeBytes() uint64 {
-	imageSize := len(l.Manifest.Contents)
+	imageSize := uint64(len(l.Manifest.Contents))
 	for _, i := range l.Images {
-		imageSize += len(i.Manifest.Contents)
+		imageSize += uint64(len(i.Manifest.Contents))
 	}
-	return uint64(imageSize)
+	return imageSize
 }
 
 // DigestRef returns the ManifestReference for this manifest's digest.