From bdb96fe0ffddd3c1b45e432d893c1de4520513e6 Mon Sep 17 00:00:00 2001 From: Brad Sickles Date: Wed, 7 Oct 2026 09:31:34 -0400 Subject: [PATCH] security: resolve Oneleet gosec findings - G402: annotate InsecureSkipVerify in docker client; it mirrors the SkipTLSVerify setting of the user's own docker context - G401/G501: annotate md5 in S3 zip upload; it feeds the Content-MD5 integrity header required by object-lock buckets - G304: annotate every os.Open/Create/ReadFile with the reason the path is trusted (caller-provided source, enumerated from baseDir, or a well-known docker config file) - G301: download dirs created 0750 instead of 0755 - Add a traversal guard to the four directory downloaders (S3, Azure Blob, GCS, Composer) so an object key containing ".." can never write outside localDir --- aws/s3/download_dir_artifact.go | 7 +++++-- aws/s3/download_zip_artifact.go | 2 +- aws/s3/upload_zip_artifact.go | 4 ++-- aws/s3/zip_pusher.go | 2 +- aws/s3_uploader.go | 2 +- azure/blob/dir_pusher.go | 9 ++++++--- azure/blob/zip_pusher.go | 4 ++-- azure/functions/deployer.go | 2 +- docker/docker_cli.go | 2 +- docker/docker_context.go | 4 ++-- gcp/composer/pusher.go | 9 ++++++--- gcp/gcs/download_dir_artifact.go | 7 +++++-- gcp/gcs/download_zip_artifact.go | 2 +- gcp/gcs/uploader.go | 2 +- 14 files changed, 35 insertions(+), 23 deletions(-) diff --git a/aws/s3/download_dir_artifact.go b/aws/s3/download_dir_artifact.go index 68f6c47..eb1dbae 100644 --- a/aws/s3/download_dir_artifact.go +++ b/aws/s3/download_dir_artifact.go @@ -32,7 +32,10 @@ func DownloadDirArtifact(ctx context.Context, infra Outputs, localDir string, ve } localPath := filepath.Join(localDir, filepath.FromSlash(relPath)) - if err := os.MkdirAll(filepath.Dir(localPath), 0755); err != nil { + if rel, err := filepath.Rel(localDir, localPath); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("refusing to write object %q outside %q", key, localDir) + } + if err := os.MkdirAll(filepath.Dir(localPath), 0750); err != nil { return fmt.Errorf("error creating directory for %q: %w", localPath, err) } @@ -44,7 +47,7 @@ func DownloadDirArtifact(ctx context.Context, infra Outputs, localDir string, ve return fmt.Errorf("error downloading %q: %w", key, err) } - file, err := os.Create(localPath) + file, err := os.Create(localPath) // #nosec G304 -- path is under the caller-provided localDir; traversal guarded above if err != nil { out.Body.Close() return fmt.Errorf("error creating local file %q: %w", localPath, err) diff --git a/aws/s3/download_zip_artifact.go b/aws/s3/download_zip_artifact.go index 6fac5d7..c98533a 100644 --- a/aws/s3/download_zip_artifact.go +++ b/aws/s3/download_zip_artifact.go @@ -23,7 +23,7 @@ func DownloadZipArtifact(ctx context.Context, infra Outputs, localPath string, v } defer out.Body.Close() - file, err := os.Create(localPath) + file, err := os.Create(localPath) // #nosec G304 -- localPath is supplied by the caller if err != nil { return fmt.Errorf("error creating local file %q: %w", localPath, err) } diff --git a/aws/s3/upload_zip_artifact.go b/aws/s3/upload_zip_artifact.go index f1dcbad..b7bc854 100644 --- a/aws/s3/upload_zip_artifact.go +++ b/aws/s3/upload_zip_artifact.go @@ -2,7 +2,7 @@ package s3 import ( "context" - "crypto/md5" + "crypto/md5" // #nosec G501 -- S3 Content-MD5 integrity header, not used for security "encoding/base64" "fmt" "github.com/aws/aws-sdk-go-v2/aws" @@ -16,7 +16,7 @@ func UploadZipArtifact(ctx context.Context, infra Outputs, content io.ReadSeeker // Calculate md5 content to add as header (necessary for s3 buckets that have object lock enabled) // After calculating, we need to reset the content stream to transmit using s3.PutObject - md5Summer := md5.New() + md5Summer := md5.New() // #nosec G401 -- S3 Content-MD5 integrity header, not used for security if _, err := io.Copy(md5Summer, content); err != nil { return fmt.Errorf("error calculating md5 hash: %w", err) } diff --git a/aws/s3/zip_pusher.go b/aws/s3/zip_pusher.go index 845dc03..2f7ea26 100644 --- a/aws/s3/zip_pusher.go +++ b/aws/s3/zip_pusher.go @@ -40,7 +40,7 @@ func (p ZipPusher) Push(ctx context.Context, source, version string) error { return fmt.Errorf("--version is required to upload artifact") } - file, err := os.Open(source) + file, err := os.Open(source) // #nosec G304 -- source is the artifact path supplied by the caller if os.IsNotExist(err) { return fmt.Errorf("source file %q does not exist", source) } else if err != nil { diff --git a/aws/s3_uploader.go b/aws/s3_uploader.go index 8c73fe4..98167aa 100644 --- a/aws/s3_uploader.go +++ b/aws/s3_uploader.go @@ -38,7 +38,7 @@ func (u *S3Uploader) UploadDir(ctx context.Context, cfg aws.Config, baseDir stri func (u *S3Uploader) uploadOne(ctx context.Context, client *transfermanager.Client, baseDir, fp string) error { localFilepath := filepath.Join(baseDir, fp) - file, err := os.Open(localFilepath) + file, err := os.Open(localFilepath) // #nosec G304 -- file enumerated from the caller-provided baseDir if err != nil { return fmt.Errorf("error opening local file %q: %w", localFilepath, err) } diff --git a/azure/blob/dir_pusher.go b/azure/blob/dir_pusher.go index 2f2763e..8ae7f54 100644 --- a/azure/blob/dir_pusher.go +++ b/azure/blob/dir_pusher.go @@ -73,7 +73,7 @@ func (p DirPusher) Push(ctx context.Context, source, version string) error { objectKey = objDir + "/" + objectKey } - file, err := os.Open(fp) + file, err := os.Open(fp) // #nosec G304 -- file enumerated from the caller-provided source dir if err != nil { return fmt.Errorf("error opening file %s: %w", fp, err) } @@ -121,7 +121,10 @@ func (p DirPusher) Pull(ctx context.Context, version string) error { relPath = strings.TrimPrefix(relPath, "/") localPath := filepath.Join(localDir, relPath) - if err := os.MkdirAll(filepath.Dir(localPath), 0755); err != nil { + if rel, err := filepath.Rel(localDir, localPath); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("refusing to write object %q outside %q", *blob.Name, localDir) + } + if err := os.MkdirAll(filepath.Dir(localPath), 0750); err != nil { return fmt.Errorf("error creating directory: %w", err) } @@ -129,7 +132,7 @@ func (p DirPusher) Pull(ctx context.Context, version string) error { if err != nil { return fmt.Errorf("error downloading %s: %w", *blob.Name, err) } - file, err := os.Create(localPath) + file, err := os.Create(localPath) // #nosec G304 -- path is under the caller-provided localDir; traversal guarded above if err != nil { resp.Body.Close() return fmt.Errorf("error creating file %s: %w", localPath, err) diff --git a/azure/blob/zip_pusher.go b/azure/blob/zip_pusher.go index e9e6b05..e8b3f8f 100644 --- a/azure/blob/zip_pusher.go +++ b/azure/blob/zip_pusher.go @@ -58,7 +58,7 @@ func (p ZipPusher) Push(ctx context.Context, source, version string) error { objectKey = strings.TrimPrefix(objectKey, "/") fmt.Fprintf(stderr, "Uploading %s to Azure Blob Storage...\n", source) - file, err := os.Open(absSource) + file, err := os.Open(absSource) // #nosec G304 -- source is the artifact path supplied by the caller if err != nil { if os.IsNotExist(err) { return fmt.Errorf("source file %q does not exist", source) @@ -93,7 +93,7 @@ func (p ZipPusher) Pull(ctx context.Context, version string) error { localPath := fmt.Sprintf("./%s-%s-%s.zip", p.AppDetails.App.Name, p.AppDetails.Env.Name, version) fmt.Fprintf(stderr, "Downloading %s from Azure Blob Storage...\n", objectKey) - file, err := os.Create(localPath) + file, err := os.Create(localPath) // #nosec G304 -- fixed filename pattern in the working dir if err != nil { return fmt.Errorf("error creating local file: %w", err) } diff --git a/azure/functions/deployer.go b/azure/functions/deployer.go index da419a8..069f46d 100644 --- a/azure/functions/deployer.go +++ b/azure/functions/deployer.go @@ -80,7 +80,7 @@ func (d Deployer) Deploy(ctx context.Context, meta app.DeployMetadata) (string, zipPath = meta.PackageMode } - zipData, err := os.ReadFile(zipPath) + zipData, err := os.ReadFile(zipPath) // #nosec G304 -- artifact path supplied by the caller if err != nil { // If we can't read a local zip, try deploying via the ARM API with the version as a reference fmt.Fprintf(stdout, "No local zip file found at %q, deploying via ARM restart...\n", zipPath) diff --git a/docker/docker_cli.go b/docker/docker_cli.go index b28e262..8562851 100644 --- a/docker/docker_cli.go +++ b/docker/docker_cli.go @@ -90,7 +90,7 @@ func clientOptions(endpoint Endpoint) ([]client.Opt, error) { func insecureHttpClient(endpoint Endpoint) (*http.Client, error) { tlsConfig := &tls.Config{ - InsecureSkipVerify: true, + InsecureSkipVerify: true, // #nosec G402 -- honours the SkipTLSVerify setting of the user's own docker context MinVersion: tls.VersionTLS12, } if endpoint.CertFile != "" && endpoint.KeyFile != "" { diff --git a/docker/docker_context.go b/docker/docker_context.go index 8768444..2503e15 100644 --- a/docker/docker_context.go +++ b/docker/docker_context.go @@ -83,7 +83,7 @@ func currentContextName() (string, error) { if dir == "" { return "", nil } - raw, err := os.ReadFile(filepath.Join(dir, "config.json")) + raw, err := os.ReadFile(filepath.Join(dir, "config.json")) // #nosec G304 -- well-known file under the docker config dir if err != nil { if errors.Is(err, fs.ErrNotExist) { return "", nil @@ -106,7 +106,7 @@ func loadContextEndpoint(name string) (Endpoint, error) { } contextDir := contextDirOf(name) - raw, err := os.ReadFile(filepath.Join(dir, "contexts", "meta", contextDir, "meta.json")) + raw, err := os.ReadFile(filepath.Join(dir, "contexts", "meta", contextDir, "meta.json")) // #nosec G304 -- well-known file under the docker config dir if err != nil { if errors.Is(err, fs.ErrNotExist) { return Endpoint{}, fmt.Errorf("docker context %q not found", name) diff --git a/gcp/composer/pusher.go b/gcp/composer/pusher.go index b49e267..5a3abd4 100644 --- a/gcp/composer/pusher.go +++ b/gcp/composer/pusher.go @@ -193,7 +193,7 @@ func (p Pusher) newStorageClient(ctx context.Context) (*storage.Client, error) { func uploadOne(ctx context.Context, bucket *storage.BucketHandle, baseDir, rel, objectKey string) error { localFilepath := filepath.Join(baseDir, rel) - file, err := os.Open(localFilepath) + file, err := os.Open(localFilepath) // #nosec G304 -- file enumerated from the caller-provided baseDir if err != nil { return fmt.Errorf("error opening local file %q: %w", localFilepath, err) } @@ -219,7 +219,10 @@ func downloadOne(ctx context.Context, bucket *storage.BucketHandle, objectName, return nil } localFilepath := filepath.Join(localDir, filepath.FromSlash(rel)) - if err := os.MkdirAll(filepath.Dir(localFilepath), 0755); err != nil { + if rel, err := filepath.Rel(localDir, localFilepath); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("refusing to write object %q outside %q", objectName, localDir) + } + if err := os.MkdirAll(filepath.Dir(localFilepath), 0750); err != nil { return fmt.Errorf("error creating local directory: %w", err) } @@ -229,7 +232,7 @@ func downloadOne(ctx context.Context, bucket *storage.BucketHandle, objectName, } defer reader.Close() - file, err := os.Create(localFilepath) + file, err := os.Create(localFilepath) // #nosec G304 -- path is under the caller-provided localDir; traversal guarded above if err != nil { return fmt.Errorf("error creating local file %q: %w", localFilepath, err) } diff --git a/gcp/gcs/download_dir_artifact.go b/gcp/gcs/download_dir_artifact.go index 5c2c0cd..b321326 100644 --- a/gcp/gcs/download_dir_artifact.go +++ b/gcp/gcs/download_dir_artifact.go @@ -49,7 +49,10 @@ func DownloadDirArtifact(ctx context.Context, infra Outputs, localDir string, ve } localPath := filepath.Join(localDir, filepath.FromSlash(relPath)) - if err := os.MkdirAll(filepath.Dir(localPath), 0755); err != nil { + if rel, err := filepath.Rel(localDir, localPath); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("refusing to write object %q outside %q", attrs.Name, localDir) + } + if err := os.MkdirAll(filepath.Dir(localPath), 0750); err != nil { return fmt.Errorf("error creating directory for %q: %w", localPath, err) } @@ -58,7 +61,7 @@ func DownloadDirArtifact(ctx context.Context, infra Outputs, localDir string, ve return fmt.Errorf("error downloading %q: %w", attrs.Name, err) } - file, err := os.Create(localPath) + file, err := os.Create(localPath) // #nosec G304 -- path is under the caller-provided localDir; traversal guarded above if err != nil { reader.Close() return fmt.Errorf("error creating local file %q: %w", localPath, err) diff --git a/gcp/gcs/download_zip_artifact.go b/gcp/gcs/download_zip_artifact.go index b0800ee..dbb4f52 100644 --- a/gcp/gcs/download_zip_artifact.go +++ b/gcp/gcs/download_zip_artifact.go @@ -31,7 +31,7 @@ func DownloadZipArtifact(ctx context.Context, infra Outputs, localPath string, v } defer reader.Close() - file, err := os.Create(localPath) + file, err := os.Create(localPath) // #nosec G304 -- localPath is supplied by the caller if err != nil { return fmt.Errorf("error creating local file %q: %w", localPath, err) } diff --git a/gcp/gcs/uploader.go b/gcp/gcs/uploader.go index a00fb7f..361b7f4 100644 --- a/gcp/gcs/uploader.go +++ b/gcp/gcs/uploader.go @@ -64,7 +64,7 @@ func (u *Uploader) uploadOne(ctx context.Context, bucket *storage.BucketHandle, objectKey := u.ObjectKeyFn(filename) localFilepath := filepath.Join(baseDir, filename) - file, err := os.Open(localFilepath) + file, err := os.Open(localFilepath) // #nosec G304 -- file enumerated from the caller-provided baseDir if err != nil { return fmt.Errorf("error opening local file %q: %w", localFilepath, err) }