From 21ae24835c3af1e5a0565c39753e55c1aeef2f63 Mon Sep 17 00:00:00 2001 From: James Fantin-Hardesty <24646452+jfantinhardesty@users.noreply.github.com> Date: Wed, 15 Jul 2026 10:52:21 -0600 Subject: [PATCH] Enable gosec security checks --- .golangci.yml | 11 +++++++++++ cmd/config.go | 2 ++ cmd/health-monitor.go | 11 ++++++++++- cmd/health-monitor_stop.go | 8 ++++++++ cmd/health-monitor_stop_all.go | 1 + cmd/log-collector.go | 1 + cmd/mount.go | 11 ++++++++++- cmd/mount_all.go | 1 + cmd/secure.go | 1 + component/azstorage/config.go | 22 ++++++++++++---------- component/file_cache/file_cache.go | 1 + 11 files changed, 58 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e10e9a8a0..77b48ce02 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,6 +6,7 @@ linters: enable: - errcheck # Errcheck is a program for checking for unchecked errors in go programs. - gocheckcompilerdirectives # Checks that go compiler directive comments (//go:) are valid. + - gosec # Inspects source code for security problems. - govet # Vet examines Go source code and reports suspicious constructs - ineffassign # Detects when assignments to existing variables are not used - misspell # Finds commonly misspelled English words in comments. @@ -22,6 +23,12 @@ linters: - suite-thelper staticcheck: checks: ["all", "-ST1003", "-ST1005"] + gosec: + excludes: + - G306 # Don't flag write file permissions + - G401 # Allow use of md5 for non-cryptographic purposes + - G501 # Allow use of md5 for non-cryptographic purposes + - G115 # Integer conversion checks are noisy exclusions: generated: lax @@ -34,6 +41,10 @@ linters: - third_party$ - builtin$ - examples$ + rules: + - path: '(.+)_test\.go' + linters: + - gosec issues: max-issues-per-linter: 0 diff --git a/cmd/config.go b/cmd/config.go index 40a3f6e5e..04f487fec 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -505,9 +505,11 @@ func (tui *appContext) buildCredentialsPage() tview.Primitive { secretLabel := "" if tui.config.storageProtocol == "azstorage" { accessLabel = "🔑 Account Name: " + //nolint:gosec // G101: UI label text; no credential value is embedded. secretLabel = "🔑 Account Key: " } else { accessLabel = "🔑 Access Key: " + //nolint:gosec // G101: UI label text; no credential value is embedded. secretLabel = "🔑 Secret Key: " } diff --git a/cmd/health-monitor.go b/cmd/health-monitor.go index bae300c36..e6b9e987e 100644 --- a/cmd/health-monitor.go +++ b/cmd/health-monitor.go @@ -109,9 +109,18 @@ var healthMonCmd = &cobra.Command{ if err != nil { return fmt.Errorf("failed to start health monitor: %w", err) } + //nolint:gosec // G204: executable path is resolved explicitly; args are passed directly without shell. hmcmd = exec.Command(path, cliParams...) } else { - hmcmd = exec.Command(hmcommon.CfuseMon, cliParams...) + monPath, err := exec.LookPath(hmcommon.CfuseMon) + if err != nil { + return fmt.Errorf( + "failed to start health monitor: failed to locate health monitor binary: %w", + err, + ) + } + //nolint:gosec // G204: executable is resolved via LookPath; args are not interpreted by a shell. + hmcmd = exec.Command(monPath, cliParams...) } cliOut, err := hmcmd.Output() if len(cliOut) > 0 { diff --git a/cmd/health-monitor_stop.go b/cmd/health-monitor_stop.go index ca949a6ef..a0002fe82 100644 --- a/cmd/health-monitor_stop.go +++ b/cmd/health-monitor_stop.go @@ -30,6 +30,7 @@ import ( "os/exec" "regexp" "runtime" + "strconv" "strings" "github.com/spf13/cobra" @@ -60,6 +61,12 @@ or 'stop all' to stop all running monitors.`, ) } + pidNum, err := strconv.Atoi(cloudfusePid) + if err != nil || pidNum <= 0 { + return fmt.Errorf("invalid cloudfuse pid %q", cloudfusePid) + } + cloudfusePid = strconv.Itoa(pidNum) + pid, err := getPid(cloudfusePid) if err != nil { return fmt.Errorf( @@ -81,6 +88,7 @@ or 'stop all' to stop all running monitors.`, // Attempts to get pid of the health monitor func getPid(cloudfusePid string) (string, error) { if runtime.GOOS == "windows" { + //nolint:gosec // G204: cloudfusePid is validated as positive integer before command construction. cliOut := exec.Command( "wmic", "process", diff --git a/cmd/health-monitor_stop_all.go b/cmd/health-monitor_stop_all.go index 63a33a07d..ca6079237 100644 --- a/cmd/health-monitor_stop_all.go +++ b/cmd/health-monitor_stop_all.go @@ -63,6 +63,7 @@ func stopAll() error { } return nil } + //nolint:gosec // G204: command and arguments are fixed literals with no external input. cliOut := exec.Command("killall", hmcommon.CfuseMon) _, err := cliOut.Output() if err != nil { diff --git a/cmd/log-collector.go b/cmd/log-collector.go index 8ce3552c5..2ad1e56d2 100644 --- a/cmd/log-collector.go +++ b/cmd/log-collector.go @@ -421,6 +421,7 @@ func createWindowsArchive(archPath string) error { if strings.Contains(relPath, "cloudfuse") && regexp.MustCompile(`\.log(?:\.\d)?$`).MatchString(relPath) { var file *os.File + //nolint:gosec // G122: path is from filepath.Walk callback, safe for operations within Walk file, err = os.Open(path) if err != nil { return err diff --git a/cmd/mount.go b/cmd/mount.go index 38f28a634..3522ad277 100644 --- a/cmd/mount.go +++ b/cmd/mount.go @@ -32,6 +32,8 @@ import ( "fmt" "io/fs" "net/http" + + //nolint:gosec // G108: pprof for runtime profiling, exposed on demand via --profiler-port _ "net/http/pprof" "os" "path/filepath" @@ -825,7 +827,14 @@ func startDynamicProfiler() { // go tool pprof http://localhost:6060/debug/pprof/profile?seconds=30 // go tool pprof http://localhost:6060/debug/pprof/block // - err := http.ListenAndServe(connStr, nil) + server := &http.Server{ + Addr: connStr, + Handler: nil, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, + } + err := server.ListenAndServe() if err != nil { log.Err("Mount::startDynamicProfiler : Failed to start dynamic profiler [%s]", err.Error()) } diff --git a/cmd/mount_all.go b/cmd/mount_all.go index 8f583b5ba..8b57f7398 100644 --- a/cmd/mount_all.go +++ b/cmd/mount_all.go @@ -423,6 +423,7 @@ func mountAllContainers( // Now that we have mount path and config file for this container fire a mount command for this one fmt.Fprintln(out, "Mounting container :", container, "to path", contMountPath) + //nolint:gosec // G204: binary path comes from current executable, args are direct argv without shell parsing. cmd := exec.Command(mountAllOpts.cloudfuseBinPath, cliParams...) var errb bytes.Buffer diff --git a/cmd/secure.go b/cmd/secure.go index 81337f150..c83eb4a47 100644 --- a/cmd/secure.go +++ b/cmd/secure.go @@ -199,6 +199,7 @@ func decryptConfigFile(saveConfig bool) ([]byte, error) { // saveToFile: Save the newly generated config file and delete the source if requested func saveToFile(configFileName string, data []byte, deleteSource bool) error { + //nolint:gosec // G703: configFileName from CLI flags marked as filename, path is user-controlled output file err := os.WriteFile(configFileName, data, 0644) if err != nil { return err diff --git a/component/azstorage/config.go b/component/azstorage/config.go index df3f4a4b7..fee0f1cf7 100644 --- a/component/azstorage/config.go +++ b/component/azstorage/config.go @@ -158,16 +158,18 @@ const DefaultMaxResultsForList int32 = 2 // https://github.com/Azure/go-autorest/blob/a46566dfcbdc41e736295f94e9f690ceaf50094a/autorest/adal/token.go#L788 // newServicePrincipalTokenFromMSI : reads them directly from env const ( - EnvAzStorageAccount = "AZURE_STORAGE_ACCOUNT" - EnvAzStorageAccountType = "AZURE_STORAGE_ACCOUNT_TYPE" - EnvAzStorageAccessKey = "AZURE_STORAGE_ACCESS_KEY" - EnvAzStorageSasToken = "AZURE_STORAGE_SAS_TOKEN" - EnvAzStorageIdentityClientId = "AZURE_STORAGE_IDENTITY_CLIENT_ID" - EnvAzStorageIdentityResourceId = "AZURE_STORAGE_IDENTITY_RESOURCE_ID" - EnvAzStorageIdentityObjectId = "AZURE_STORAGE_IDENTITY_OBJECT_ID" - EnvAzStorageSpnTenantId = "AZURE_STORAGE_SPN_TENANT_ID" - EnvAzStorageSpnClientId = "AZURE_STORAGE_SPN_CLIENT_ID" - EnvAzStorageSpnClientSecret = "AZURE_STORAGE_SPN_CLIENT_SECRET" + EnvAzStorageAccount = "AZURE_STORAGE_ACCOUNT" + EnvAzStorageAccountType = "AZURE_STORAGE_ACCOUNT_TYPE" + EnvAzStorageAccessKey = "AZURE_STORAGE_ACCESS_KEY" + EnvAzStorageSasToken = "AZURE_STORAGE_SAS_TOKEN" + EnvAzStorageIdentityClientId = "AZURE_STORAGE_IDENTITY_CLIENT_ID" + EnvAzStorageIdentityResourceId = "AZURE_STORAGE_IDENTITY_RESOURCE_ID" + EnvAzStorageIdentityObjectId = "AZURE_STORAGE_IDENTITY_OBJECT_ID" + EnvAzStorageSpnTenantId = "AZURE_STORAGE_SPN_TENANT_ID" + EnvAzStorageSpnClientId = "AZURE_STORAGE_SPN_CLIENT_ID" + //nolint:gosec // G101: constant is an environment variable name, not a hardcoded secret value. + EnvAzStorageSpnClientSecret = "AZURE_STORAGE_SPN_CLIENT_SECRET" + //nolint:gosec // G101: constant is an environment variable name, not a hardcoded secret value. EnvAzStorageSpnOAuthTokenFilePath = "AZURE_OAUTH_TOKEN_FILE" EnvAzStorageSpnWorkloadIdentityToken = "WORKLOAD_IDENTITY_TOKEN" EnvAzStorageAadEndpoint = "AZURE_STORAGE_AAD_ENDPOINT" diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 99ca6580c..fbe2866ca 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1007,6 +1007,7 @@ func (fc *FileCache) RenameDir(options internal.RenameDirOptions) error { } else { log.Debug("FileCache::RenameDir : Creating local destination directory %s", newPath) // create the new directory + //nolint:gosec // G122: newPath is from filepath.WalkDir callback, safe for operations within WalkDir mkdirErr := os.MkdirAll(newPath, fc.defaultPermission) if mkdirErr != nil { // log any error but do nothing about it