From 0256e09031323d3e471c1b6b5e08a2d21dcfc51b Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Thu, 13 Feb 2025 16:49:40 -0800 Subject: [PATCH] lint: Clean up golangci-lint v1.60 complaints This cleans up the linting results by adding checks for integer underflow/overflow in several places, suppressing the error in places where it has been checked, or fixing the types when possible. --- cmd/gen-manifests/main.go | 12 ++++++++++-- cmd/gen-manifests/workerqueue.go | 2 +- cmd/osbuild-worker-executor/handler_build.go | 2 +- cmd/osbuild-worker/config.go | 3 ++- cmd/osbuild-worker/jobimpl-file-resolve.go | 2 +- cmd/osbuild-worker/jobimpl-koji-finalize.go | 18 ++++++++++++++++-- cmd/osbuild-worker/main.go | 2 +- internal/blueprint/fsnode_customizations.go | 4 ++++ internal/boot/netns.go | 2 ++ internal/cloudapi/v2/handler.go | 6 ++++++ internal/upload/koji/koji.go | 6 +++--- 11 files changed, 47 insertions(+), 12 deletions(-) diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index 624f1bf0d..a09bd8639 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -13,6 +13,7 @@ import ( "flag" "fmt" "io" + "math" "os" "path/filepath" "sort" @@ -446,9 +447,9 @@ func mergeOverrides(base, overrides composeRequest) composeRequest { func main() { // common args var outputDir, cacheRoot string - var nWorkers int + var nWorkers uint flag.StringVar(&outputDir, "output", "test/data/manifests/", "manifest store directory") - flag.IntVar(&nWorkers, "workers", 16, "number of workers to run concurrently") + flag.UintVar(&nWorkers, "workers", 16, "number of workers to run concurrently") flag.StringVar(&cacheRoot, "cache", "/tmp/rpmmd", "rpm metadata cache directory") // manifest selection args @@ -459,6 +460,11 @@ func main() { flag.Parse() + // nWorkers cannot be larger than uint32 + if nWorkers > math.MaxUint32 { + panic(fmt.Sprintf("--workers must be %d or less.", math.MaxUint32)) + } + seedArg := int64(0) darm := readRepos() distroFac := distrofactory.NewDefault() @@ -536,6 +542,8 @@ func main() { nJobs := len(jobs) fmt.Printf("Collected %d jobs\n", nJobs) + // nWorkers has been tested to be <= math.MaxUint32 + /* #nosec G115 */ wq := newWorkerQueue(uint32(nWorkers), uint32(nJobs)) wq.start() fmt.Printf("Initialised %d workers\n", nWorkers) diff --git a/cmd/gen-manifests/workerqueue.go b/cmd/gen-manifests/workerqueue.go index 6b2d4413a..d7fe8751d 100644 --- a/cmd/gen-manifests/workerqueue.go +++ b/cmd/gen-manifests/workerqueue.go @@ -94,7 +94,7 @@ func (wq *workerQueue) startMessagePrinter() { var msglen int for msg := range wq.msgQueue { // clear previous line (avoids leftover trailing characters from progress) - fmt.Printf(strings.Repeat(" ", msglen) + "\r") + fmt.Print(strings.Repeat(" ", msglen) + "\r") fmt.Println(msg) msglen, _ = fmt.Printf(" == Jobs == Queue: %4d Active: %4d Total: %4d\r", len(wq.jobQueue), wq.activeWorkers, wq.njobs) } diff --git a/cmd/osbuild-worker-executor/handler_build.go b/cmd/osbuild-worker-executor/handler_build.go index 65d338e5f..f2dbabfcc 100644 --- a/cmd/osbuild-worker-executor/handler_build.go +++ b/cmd/osbuild-worker-executor/handler_build.go @@ -202,7 +202,7 @@ func handleIncludedSources(atar *tar.Reader, buildDir string) error { // this assume "well" behaving tars, i.e. all dirs that lead // up to the tar are included etc - mode := os.FileMode(hdr.Mode) + mode := hdr.FileInfo().Mode() switch hdr.Typeflag { case tar.TypeDir: if err := os.Mkdir(target, mode); err != nil { diff --git a/cmd/osbuild-worker/config.go b/cmd/osbuild-worker/config.go index ac3dae592..fc4ce8f43 100644 --- a/cmd/osbuild-worker/config.go +++ b/cmd/osbuild-worker/config.go @@ -3,6 +3,7 @@ package main import ( "fmt" "os" + "time" "github.com/BurntSushi/toml" "github.com/osbuild/osbuild-composer/internal/upload/azure" @@ -20,7 +21,7 @@ type kerberosConfig struct { type kojiServerConfig struct { Kerberos *kerberosConfig `toml:"kerberos,omitempty"` - RelaxTimeoutFactor uint `toml:"relax_timeout_factor"` + RelaxTimeoutFactor time.Duration `toml:"relax_timeout_factor"` } type gcpConfig struct { diff --git a/cmd/osbuild-worker/jobimpl-file-resolve.go b/cmd/osbuild-worker/jobimpl-file-resolve.go index bdd718718..0886005e1 100644 --- a/cmd/osbuild-worker/jobimpl-file-resolve.go +++ b/cmd/osbuild-worker/jobimpl-file-resolve.go @@ -28,7 +28,7 @@ func (impl *FileResolveJobImpl) Run(job worker.Job) error { } } - if result.Results == nil || len(result.Results) == 0 { + if len(result.Results) == 0 { logWithId.Infof("Resolving file contents failed: %v", err) result.JobError = clienterrors.New( clienterrors.ErrorRemoteFileResolution, diff --git a/cmd/osbuild-worker/jobimpl-koji-finalize.go b/cmd/osbuild-worker/jobimpl-koji-finalize.go index bdec7107e..c3e00c693 100644 --- a/cmd/osbuild-worker/jobimpl-koji-finalize.go +++ b/cmd/osbuild-worker/jobimpl-koji-finalize.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "math" "net/url" "time" @@ -102,7 +103,14 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { // Fail the Koji build if the job error is set and the necessary // information to identify the job are available. if kojiFinalizeJobResult.JobError != nil && initArgs != nil { - err = impl.kojiFail(args.Server, int(initArgs.BuildID), initArgs.Token) + /* #nosec G115 */ + buildID := int(initArgs.BuildID) + // Make sure that signed integer conversion didn't underflow + if buildID < 0 { + logWithId.Errorf("BuildID integer underflow: %d", initArgs.BuildID) + return + } + err = impl.kojiFail(args.Server, buildID, initArgs.Token) if err != nil { logWithId.Errorf("Failing Koji job failed: %v", err) } @@ -305,13 +313,19 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { } } + // Make sure StartTime cannot overflow the int64 conversion + if args.StartTime > math.MaxInt64 { + return fmt.Errorf("StartTime integer overflow: %d", args.StartTime) + } + /* #nosec G115 */ + startTime := int64(args.StartTime) build := koji.Build{ BuildID: initArgs.BuildID, TaskID: args.TaskID, Name: args.Name, Version: args.Version, Release: args.Release, - StartTime: int64(args.StartTime), + StartTime: startTime, EndTime: time.Now().Unix(), Extra: koji.BuildExtra{ TypeInfo: koji.TypeInfoBuild{ diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go index e7ec1c8dc..a9dc1e96f 100644 --- a/cmd/osbuild-worker/main.go +++ b/cmd/osbuild-worker/main.go @@ -39,7 +39,7 @@ type connectionConfig struct { type kojiServer struct { creds koji.GSSAPICredentials - relaxTimeoutFactor uint + relaxTimeoutFactor time.Duration } // Represents the implementation of a job type as defined by the worker API. diff --git a/internal/blueprint/fsnode_customizations.go b/internal/blueprint/fsnode_customizations.go index f83695b96..d1a240910 100644 --- a/internal/blueprint/fsnode_customizations.go +++ b/internal/blueprint/fsnode_customizations.go @@ -144,6 +144,8 @@ func (d DirectoryCustomization) ToFsNodeDirectory() (*fsnode.Directory, error) { if err != nil { return nil, fmt.Errorf("invalid mode %s: %v", d.Mode, err) } + // modeNum is parsed as an unsigned 32 bit int + /* #nosec G115 */ mode = common.ToPtr(os.FileMode(modeNum)) } @@ -302,6 +304,8 @@ func (f FileCustomization) ToFsNodeFile() (*fsnode.File, error) { if err != nil { return nil, fmt.Errorf("invalid mode %s: %v", f.Mode, err) } + // modeNum is parsed as an unsigned 32 bit int + /* #nosec G115 */ mode = common.ToPtr(os.FileMode(modeNum)) } diff --git a/internal/boot/netns.go b/internal/boot/netns.go index ec693eb0a..e495c9453 100644 --- a/internal/boot/netns.go +++ b/internal/boot/netns.go @@ -73,6 +73,8 @@ func newNetworkNamespace() (NetNS, error) { return "", fmt.Errorf("cannot unshare the network namespace") } defer func() { + // The Fd() is actually an int cast into a uintptr, so casting back to an int is fine + /* #nosec G115 */ err = unix.Setns(int(oldNS.Fd()), syscall.CLONE_NEWNET) if err != nil { // We cannot return to the original namespace. diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index ed8212ab8..6470ee6d1 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -464,7 +464,13 @@ func (h *apiHandlers) getJobIDComposeStatus(jobId uuid.UUID) (ComposeStatus, err ImageStatuses: &buildJobStatuses, KojiStatus: &KojiStatus{}, } + /* #nosec G115 */ buildID := int(initResult.BuildID) + // Make sure signed integer conversion didn't underflow + if buildID < 0 { + err := fmt.Errorf("BuildID integer underflow: %d", initResult.BuildID) + return ComposeStatus{}, HTTPErrorWithInternal(ErrorMalformedOSBuildJobResult, err) + } if buildID != 0 { response.KojiStatus.BuildId = &buildID } diff --git a/internal/upload/koji/koji.go b/internal/upload/koji/koji.go index 0ced02d32..653836b6d 100644 --- a/internal/upload/koji/koji.go +++ b/internal/upload/koji/koji.go @@ -574,7 +574,7 @@ func GSSAPICredentialsFromEnv() (*GSSAPICredentials, error) { }, nil } -func CreateKojiTransport(relaxTimeout uint) http.RoundTripper { +func CreateKojiTransport(relaxTimeout time.Duration) http.RoundTripper { // Koji for some reason needs TLS renegotiation enabled. // Clone the default http rt and enable renegotiation. rt := CreateRetryableTransport() @@ -588,9 +588,9 @@ func CreateKojiTransport(relaxTimeout uint) http.RoundTripper { // Relax timeouts a bit if relaxTimeout > 0 { - transport.TLSHandshakeTimeout *= time.Duration(relaxTimeout) + transport.TLSHandshakeTimeout *= relaxTimeout transport.DialContext = (&net.Dialer{ - Timeout: 30 * time.Second * time.Duration(relaxTimeout), + Timeout: 30 * time.Second * relaxTimeout, KeepAlive: 30 * time.Second, }).DialContext }