Merge pull request #2061 from authzed/fix-security-errors

Fix security errors in lint steps

.github/workflows/nightly.yaml

@@ -30,7 +30,8 @@ jobs:
       - uses: "goreleaser/goreleaser-action@v6"
         with:
           distribution: "goreleaser-pro"
-          version: "latest"
+          # Pinned because of a regression in 2.3.0
+          version: "2.2.0"
           args: "release -f .goreleaser.nightly.yml --clean --nightly"
         env:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"

.github/workflows/release.yaml

@@ -33,7 +33,8 @@ jobs:
       - uses: "goreleaser/goreleaser-action@v6"
         with:
           distribution: "goreleaser-pro"
-          version: "latest"
+          # Pinned because of a regression in 2.3.0
+          version: &goreleaser_version "2.2.0"
           args: "release --clean --config=.goreleaser.windows.yml"
         env:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
@@ -69,7 +70,7 @@ jobs:
       - uses: "goreleaser/goreleaser-action@v6"
         with:
           distribution: "goreleaser-pro"
-          version: "latest"
+          version: &goreleaser_version
           args: "release --clean"
         env:
           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"

.github/workflows/security.yaml

@@ -71,7 +71,8 @@ jobs:
         id: "goreleaser"
         with:
           distribution: "goreleaser-pro"
-          version: "latest"
+          # Pinned because of a regression in 2.2.0
+          version: "2.2.0"
           args: "release --clean --split --snapshot --single-target --skip=chocolatey"
         env:
           GORELEASER_KEY: "${{ secrets.GORELEASER_KEY }}"

.goreleaser.nightly.yml

@@ -1,4 +1,5 @@
 ---
+version: 2
 builds:
   - main: "./cmd/spicedb"
     env:
@@ -96,6 +97,6 @@ docker_manifests:
 checksum:
   name_template: "checksums.txt"
 snapshot:
-  name_template: "{{ incpatch .Version }}-next"
+  version_template: "{{ incpatch .Version }}-next"
 nightly:
-  name_template: "{{ incpatch .Version }}-{{ .ShortCommit }}"
+  version_template: "{{ incpatch .Version }}-{{ .ShortCommit }}"

.goreleaser.windows.yml

@@ -1,4 +1,5 @@
 ---
+version: 2
 git:
   tag_sort: "-version:creatordate"
   prerelease_suffix: "-"
@@ -32,7 +33,6 @@ chocolateys:
     owners: "AuthZed, Inc"
     title: "SpiceDB"
     project_url: "https://github.com/authzed/spicedb"
-    use: "archive"
     url_template: "https://github.com/authzed/spicedb/releases/download/{{ .Tag }}/{{ .ArtifactName }}"
     icon_url: "https://authzed.com/favicon.svg"
     copyright: "2024 AuthZed, Inc"
@@ -50,6 +50,6 @@ chocolateys:
 checksum:
   name_template: "checksums.txt"
 snapshot:
-  name_template: "{{ incpatch .Version }}-next"
+  version_template: "{{ incpatch .Version }}-next"
 nightly:
-  name_template: "{{ incpatch .Version }}-{{ .ShortCommit }}"
+  version_template: "{{ incpatch .Version }}-{{ .ShortCommit }}"

.goreleaser.yml

@@ -1,4 +1,6 @@
+# yaml-language-server: $schema=https://goreleaser.com/static/schema.json
 ---
+version: 2
 git:
   tag_sort: "-version:creatordate"
   prerelease_suffix: "-"
@@ -189,12 +191,12 @@ docker_manifests:
 checksum:
   name_template: "checksums.txt"
 snapshot:
-  name_template: "{{ incpatch .Version }}-next"
+  version_template: "{{ incpatch .Version }}-next"
 changelog:
   use: "github-native"
   sort: "asc"
 nightly:
-  name_template: "{{ incpatch .Version }}-{{ .ShortCommit }}"
+  version_template: "{{ incpatch .Version }}-{{ .ShortCommit }}"
 release:
   draft: false
   prerelease: "auto"

Dockerfile

@@ -1,10 +1,10 @@
-FROM golang:1.23.0-alpine3.20 AS spicedb-builder
+FROM golang:1.23.1-alpine3.20 AS spicedb-builder
 WORKDIR /go/src/app
 RUN apk update && apk add --no-cache git
 COPY . .
 RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg/mod CGO_ENABLED=0 go build -v ./cmd/...
 
-FROM golang:1.23.0-alpine3.20 AS health-probe-builder
+FROM golang:1.23.1-alpine3.20 AS health-probe-builder
 WORKDIR /go/src/app
 RUN apk update && apk add --no-cache git
 RUN git clone https://github.com/grpc-ecosystem/grpc-health-probe.git

Dockerfile.release

@@ -1,7 +1,7 @@
 # vim: syntax=dockerfile
 ARG BASE=cgr.dev/chainguard/static:latest
 
-FROM golang:1.23.0-alpine3.20 AS health-probe-builder
+FROM golang:1.23.1-alpine3.20 AS health-probe-builder
 WORKDIR /go/src/app
 RUN apk update && apk add --no-cache git
 RUN git clone https://github.com/grpc-ecosystem/grpc-health-probe.git

e2e/go.mod

@@ -1,6 +1,6 @@
 module github.com/authzed/spicedb/e2e
 
-go 1.22.4
+go 1.22.7
 
 require (
 	github.com/authzed/authzed-go v0.15.0
@@ -33,6 +33,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/sts v1.30.5 // indirect
 	github.com/aws/smithy-go v1.20.4 // indirect
 	github.com/benbjohnson/clock v1.3.5 // indirect
+	github.com/ccoveille/go-safecast v1.1.0 // indirect
 	github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect
 	github.com/creasty/defaults v1.8.0 // indirect
 	github.com/dave/jennifer v1.6.1 // indirect
@@ -54,7 +55,6 @@ require (
 	github.com/planetscale/vtprotobuf v0.6.1-0.20240409071808-615f978279ca // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/rs/zerolog v1.33.0 // indirect
-	github.com/samber/lo v1.47.0 // indirect
 	github.com/shopspring/decimal v1.4.0 // indirect
 	github.com/stoewer/go-strcase v1.3.0 // indirect
 	go.opentelemetry.io/otel v1.29.0 // indirect

e2e/go.sum

@@ -73,6 +73,8 @@ github.com/bits-and-blooms/bloom/v3 v3.7.0 h1:VfknkqV4xI+PsaDIsoHueyxVDZrfvMn56j
 github.com/bits-and-blooms/bloom/v3 v3.7.0/go.mod h1:VKlUSvp0lFIYqxJjzdnSsZEw4iHb1kOL2tfHTgyJBHg=
 github.com/brianvoe/gofakeit/v6 v6.23.2 h1:lVde18uhad5wII/f5RMVFLtdQNE0HaGFuBUXmYKk8i8=
 github.com/brianvoe/gofakeit/v6 v6.23.2/go.mod h1:Ow6qC71xtwm79anlwKRlWZW6zVq9D2XHE4QSSMP/rU8=
+github.com/ccoveille/go-safecast v1.1.0 h1:iHKNWaZm+OznO7Eh6EljXPjGfGQsSfa6/sxPlIEKO+g=
+github.com/ccoveille/go-safecast v1.1.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8=
 github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
 github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=

go.mod

@@ -1,6 +1,6 @@
 module github.com/authzed/spicedb
 
-go 1.22.4
+go 1.22.7
 
 require (
 	buf.build/gen/go/prometheus/prometheus/protocolbuffers/go v1.34.2-20240802094132-5b212ab78fb7.2
@@ -136,6 +136,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/sts v1.30.5 // indirect
 	github.com/aws/smithy-go v1.20.4 // indirect
 	github.com/bombsimon/wsl/v4 v4.4.1 // indirect
+	github.com/ccoveille/go-safecast v1.1.0 // indirect
 	github.com/cilium/ebpf v0.9.1 // indirect
 	github.com/containerd/cgroups/v3 v3.0.1 // indirect
 	github.com/coreos/go-systemd/v22 v22.5.0 // indirect

go.sum

@@ -770,6 +770,8 @@ github.com/catenacyber/perfsprint v0.7.1 h1:PGW5G/Kxn+YrN04cRAZKC+ZuvlVwolYMrIyy
 github.com/catenacyber/perfsprint v0.7.1/go.mod h1:/wclWYompEyjUD2FuIIDVKNkqz7IgBIWXIH3V0Zol50=
 github.com/ccojocar/zxcvbn-go v1.0.2 h1:na/czXU8RrhXO4EZme6eQJLR4PzcGsahsBOAwU6I3Vg=
 github.com/ccojocar/zxcvbn-go v1.0.2/go.mod h1:g1qkXtUSvHP8lhHp5GrSmTz6uWALGRMQdw6Qnz/hi60=
+github.com/ccoveille/go-safecast v1.1.0 h1:iHKNWaZm+OznO7Eh6EljXPjGfGQsSfa6/sxPlIEKO+g=
+github.com/ccoveille/go-safecast v1.1.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8=
 github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
 github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=

internal/datastore/common/changes.go

@@ -6,6 +6,8 @@ import (
 
 	"golang.org/x/exp/maps"
 
+	"github.com/ccoveille/go-safecast"
+
 	log "github.com/authzed/spicedb/internal/logging"
 	"github.com/authzed/spicedb/pkg/datastore"
 	core "github.com/authzed/spicedb/pkg/proto/core/v1"
@@ -118,7 +120,12 @@ func (ch *Changes[R, K]) adjustByteSize(item sized, delta int) error {
 		return spiceerrors.MustBugf("byte size underflow")
 	}
 
-	if ch.currentByteSize > int64(ch.maxByteSize) {
+	currentByteSize, err := safecast.ToUint64(ch.currentByteSize)
+	if err != nil {
+		return spiceerrors.MustBugf("could not cast currentByteSize to uint64: %v", err)
+	}
+
+	if currentByteSize > ch.maxByteSize {
 		return datastore.NewMaximumChangesSizeExceededError(ch.maxByteSize)
 	}
 

internal/datastore/common/gc_test.go

@@ -68,7 +68,7 @@ func (gc *fakeGC) DeleteBeforeTx(_ context.Context, rev datastore.Revision) (Del
 
 	revInt := rev.(revisions.TransactionIDRevision).TransactionID()
 
-	return gc.deleter.DeleteBeforeTx(int64(revInt))
+	return gc.deleter.DeleteBeforeTx(revInt)
 }
 
 func (gc *fakeGC) HasGCRun() bool {
@@ -101,22 +101,22 @@ func (gc *fakeGC) GetMetrics() gcMetrics {
 
 // Allows specifying different deletion behaviors for tests
 type gcDeleter interface {
-	DeleteBeforeTx(revision int64) (DeletionCounts, error)
+	DeleteBeforeTx(revision uint64) (DeletionCounts, error)
 }
 
 // Always error trying to perform a delete
 type alwaysErrorDeleter struct{}
 
-func (alwaysErrorDeleter) DeleteBeforeTx(_ int64) (DeletionCounts, error) {
+func (alwaysErrorDeleter) DeleteBeforeTx(_ uint64) (DeletionCounts, error) {
 	return DeletionCounts{}, fmt.Errorf("delete error")
 }
 
 // Only error on specific revisions
 type revisionErrorDeleter struct {
-	errorOnRevisions []int64
+	errorOnRevisions []uint64
 }
 
-func (d revisionErrorDeleter) DeleteBeforeTx(revision int64) (DeletionCounts, error) {
+func (d revisionErrorDeleter) DeleteBeforeTx(revision uint64) (DeletionCounts, error) {
 	if slices.Contains(d.errorOnRevisions, revision) {
 		return DeletionCounts{}, fmt.Errorf("delete error")
 	}
@@ -178,7 +178,7 @@ func TestGCFailureBackoffReset(t *testing.T) {
 		// Error on revisions 1 - 5, giving the exponential
 		// backoff enough time to fail the test if the interval
 		// is not reset properly.
-		errorOnRevisions: []int64{1, 2, 3, 4, 5},
+		errorOnRevisions: []uint64{1, 2, 3, 4, 5},
 	})
 
 	ctx, cancel := context.WithCancel(context.Background())

internal/datastore/common/sql.go

@@ -551,12 +551,17 @@ func (tqs QueryExecutor) ExecuteQuery(
 		query = query.After(queryOpts.After, queryOpts.Sort)
 	}
 
-	limit := math.MaxInt
+	var limit uint64
+	// NOTE: we use a uint here because it lines up with the
+	// assignments in this function, but we set it to MaxInt64
+	// because that's the biggest value that postgres and friends
+	// treat as valid.
+	limit = math.MaxInt64
 	if queryOpts.Limit != nil {
-		limit = int(*queryOpts.Limit)
+		limit = *queryOpts.Limit
 	}
 
-	toExecute := query.limit(uint64(limit))
+	toExecute := query.limit(limit)
 	sql, args, err := toExecute.queryBuilder.ToSql()
 	if err != nil {
 		return nil, err
@@ -567,7 +572,8 @@ func (tqs QueryExecutor) ExecuteQuery(
 		return nil, err
 	}
 
-	if len(queryTuples) > limit {
+	lenQueryTuples := uint64(len(queryTuples))
+	if lenQueryTuples > limit {
 		queryTuples = queryTuples[:limit]
 	}
 

internal/datastore/crdb/crdb.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/IBM/pgxpoolprometheus"
 	sq "github.com/Masterminds/squirrel"
+	"github.com/ccoveille/go-safecast"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/jackc/pgx/v5/pgxpool"
@@ -28,6 +29,7 @@ import (
 	log "github.com/authzed/spicedb/internal/logging"
 	"github.com/authzed/spicedb/pkg/datastore"
 	"github.com/authzed/spicedb/pkg/datastore/options"
+	"github.com/authzed/spicedb/pkg/spiceerrors"
 )
 
 func init() {
@@ -99,13 +101,19 @@ func newCRDBDatastore(ctx context.Context, url string, options ...Option) (datas
 	if err != nil {
 		return nil, common.RedactAndLogSensitiveConnString(ctx, errUnableToInstantiate, err, url)
 	}
-	config.readPoolOpts.ConfigurePgx(readPoolConfig)
+	err = config.readPoolOpts.ConfigurePgx(readPoolConfig)
+	if err != nil {
+		return nil, common.RedactAndLogSensitiveConnString(ctx, errUnableToInstantiate, err, url)
+	}
 
 	writePoolConfig, err := pgxpool.ParseConfig(url)
 	if err != nil {
 		return nil, common.RedactAndLogSensitiveConnString(ctx, errUnableToInstantiate, err, url)
 	}
-	config.writePoolOpts.ConfigurePgx(writePoolConfig)
+	err = config.writePoolOpts.ConfigurePgx(writePoolConfig)
+	if err != nil {
+		return nil, common.RedactAndLogSensitiveConnString(ctx, errUnableToInstantiate, err, url)
+	}
 
 	initCtx, initCancel := context.WithTimeout(context.Background(), 5*time.Minute)
 	defer initCancel()
@@ -414,8 +422,14 @@ func (cds *crdbDatastore) ReadyState(ctx context.Context) (datastore.ReadyState,
 	if writeMin > 0 {
 		writeMin--
 	}
-	writeTotal := uint32(cds.writePool.Stat().TotalConns())
-	readTotal := uint32(cds.readPool.Stat().TotalConns())
+	writeTotal, err := safecast.ToUint32(cds.writePool.Stat().TotalConns())
+	if err != nil {
+		return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast writeTotal to uint32: %v", err)
+	}
+	readTotal, err := safecast.ToUint32(cds.readPool.Stat().TotalConns())
+	if err != nil {
+		return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast readTotal to uint32: %v", err)
+	}
 	if writeTotal < writeMin || readTotal < readMin {
 		return datastore.ReadyState{
 			Message: fmt.Sprintf(

internal/datastore/crdb/pool/balancer.go

@@ -3,11 +3,13 @@ package pool
 import (
 	"context"
 	"hash/maphash"
+	"math"
 	"math/rand"
 	"slices"
 	"strconv"
 	"time"
 
+	"github.com/ccoveille/go-safecast"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgxpool"
 	"github.com/prometheus/client_golang/prometheus"
@@ -84,7 +86,15 @@ type nodeConnectionBalancer[P balancePoolConn[C], C balanceConn] struct {
 // newNodeConnectionBalancer is generic over underlying connection types for
 // testing purposes. Callers should use the exported NewNodeConnectionBalancer.
 func newNodeConnectionBalancer[P balancePoolConn[C], C balanceConn](pool balanceablePool[P, C], healthTracker *NodeHealthTracker, interval time.Duration) *nodeConnectionBalancer[P, C] {
-	seed := int64(new(maphash.Hash).Sum64())
+	seed := int64(0)
+	for seed == 0 {
+		// Sum64 returns a uint64, and safecast will return 0 if it's not castable,
+		// which will happen about half the time (?). We just keep running it until
+		// we get a seed that fits in the box.
+		// Subtracting math.MaxInt64 should mean that we retain the entire range of
+		// possible values.
+		seed, _ = safecast.ToInt64(new(maphash.Hash).Sum64() - math.MaxInt64)
+	}
 	return &nodeConnectionBalancer[P, C]{
 		ticker:        time.NewTicker(interval),
 		sem:           semaphore.NewWeighted(1),
@@ -147,7 +157,9 @@ func (p *nodeConnectionBalancer[P, C]) mustPruneConnections(ctx context.Context)
 		}
 	}
 
-	nodeCount := uint32(p.healthTracker.HealthyNodeCount())
+	// It's highly unlikely that we'll ever have an overflow in
+	// this context, so we cast directly.
+	nodeCount, _ := safecast.ToUint32(p.healthTracker.HealthyNodeCount())
 	if nodeCount == 0 {
 		nodeCount = 1
 	}
@@ -203,7 +215,7 @@ func (p *nodeConnectionBalancer[P, C]) mustPruneConnections(ctx context.Context)
 		// it's possible for the difference in connections between nodes to differ by up to
 		// the number of nodes.
 		if p.healthTracker.HealthyNodeCount() == 0 ||
-			uint32(i) < p.pool.MaxConns()%uint32(p.healthTracker.HealthyNodeCount()) {
+			i < int(p.pool.MaxConns())%p.healthTracker.HealthyNodeCount() {
 			perNodeMax++
 		}