addressed the gosec security findings (#80)

* addressed the gosec security findings

* fixed the issue

* adding go.work for gosec

.github/workflows/pr.yml

@@ -35,3 +35,7 @@ jobs:
       - name: Run tests
         working-directory: ${{ github.workspace }}/src
         run: go test -v ./...
+      - name: Run Gosec Security Scanner
+        uses: securego/gosec@master
+        with:
+          args: ./...

.vscode/launch.json

@@ -20,6 +20,19 @@
       // Comment out if you do not need to setup environment variables for the module
       "envFile": "${workspaceFolder}/.env",
       "args": []
+    },
+    {
+      "name": "Parallels Desktop Tester Debug",
+      "type": "go",
+      "request": "launch",
+      "mode": "debug",
+      "program": "${workspaceFolder}/src/main.go",
+      // Comment out if you do not need to setup environment variables for the module
+      "envFile": "${workspaceFolder}/.env",
+      "args": [
+        "test",
+        "catalog-providers"
+      ]
     }
   ]
 }

go.work

@@ -0,0 +1,3 @@
+go 1.21
+
+use ./src

src/catalog/main.go

@@ -229,9 +229,9 @@ func (s *CatalogManifestService) getPackFilename(name string) string {
 func (s *CatalogManifestService) compressMachine(ctx basecontext.ApiContext, path string, machineFileName string, destination string) (string, error) {
 	startingTime := time.Now()
 	tarFilename := machineFileName
-	tarFilePath := filepath.Join(destination, tarFilename)
+	tarFilePath := filepath.Join(destination, filepath.Clean(tarFilename))
 
-	tarFile, err := os.Create(tarFilePath)
+	tarFile, err := os.Create(filepath.Clean(tarFilePath))
 	if err != nil {
 		return "", err
 	}
@@ -255,8 +255,8 @@ func (s *CatalogManifestService) compressMachine(ctx basecontext.ApiContext, pat
 	}
 
 	compressed := 1
-	err = filepath.Walk(path, func(filePath string, info os.FileInfo, err error) error {
-		ctx.LogInfo("[%v/%v] Compressing file %v", compressed, countFiles, filePath)
+	err = filepath.Walk(path, func(machineFilePath string, info os.FileInfo, err error) error {
+		ctx.LogInfo("[%v/%v] Compressing file %v", compressed, countFiles, machineFilePath)
 		compressed += 1
 		if err != nil {
 			return err
@@ -267,13 +267,13 @@ func (s *CatalogManifestService) compressMachine(ctx basecontext.ApiContext, pat
 			return nil
 		}
 
-		f, err := os.Open(filePath)
+		f, err := os.Open(filepath.Clean(machineFilePath))
 		if err != nil {
 			return err
 		}
 		defer f.Close()
 
-		relPath := strings.TrimPrefix(filePath, path)
+		relPath := strings.TrimPrefix(machineFilePath, path)
 		hdr := &tar.Header{
 			Name: relPath,
 			Mode: int64(info.Mode()),
@@ -296,9 +296,9 @@ func (s *CatalogManifestService) compressMachine(ctx basecontext.ApiContext, pat
 	return tarFilePath, nil
 }
 
-func (s *CatalogManifestService) decompressMachine(ctx basecontext.ApiContext, filePath string, destination string) error {
+func (s *CatalogManifestService) decompressMachine(ctx basecontext.ApiContext, machineFilePath string, destination string) error {
 	staringTime := time.Now()
-	tarFile, err := os.Open(filePath)
+	tarFile, err := os.Open(filepath.Clean(machineFilePath))
 	if err != nil {
 		return err
 	}
@@ -315,36 +315,40 @@ func (s *CatalogManifestService) decompressMachine(ctx basecontext.ApiContext, f
 			return err
 		}
 
-		filePath := filepath.Join(destination, header.Name)
+		machineFilePath, err := helpers.SanitizeArchivePath(destination, header.Name)
+		if err != nil {
+			return err
+		}
+
 		// Creating the basedir if it does not exist
-		baseDir := filepath.Dir(filePath)
+		baseDir := filepath.Dir(machineFilePath)
 		if _, err := os.Stat(baseDir); os.IsNotExist(err) {
-			if err := os.MkdirAll(baseDir, 0755); err != nil {
+			if err := os.MkdirAll(baseDir, 0o750); err != nil {
 				return err
 			}
 		}
 
 		switch header.Typeflag {
 		case tar.TypeDir:
-			if _, err := os.Stat(filePath); os.IsNotExist(err) {
-				if err := os.MkdirAll(filePath, os.FileMode(header.Mode)); err != nil {
+			if _, err := os.Stat(machineFilePath); os.IsNotExist(err) {
+				if err := os.MkdirAll(machineFilePath, os.FileMode(header.Mode)); err != nil {
 					return err
 				}
 			}
 		case tar.TypeReg:
-			file, err := os.OpenFile(filePath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, os.FileMode(header.Mode))
+			file, err := os.OpenFile(filepath.Clean(machineFilePath), os.O_CREATE|os.O_TRUNC|os.O_WRONLY, os.FileMode(header.Mode))
 			if err != nil {
 				return err
 			}
 			defer file.Close()
 
-			if _, err := io.Copy(file, tarReader); err != nil {
+			if err := helpers.CopyTarChunks(file, tarReader); err != nil {
 				return err
 			}
 		}
 	}
 
 	endingTime := time.Now()
-	ctx.LogInfo("Finished decompressing machine from %s to %s, in %v", filePath, destination, endingTime.Sub(staringTime))
+	ctx.LogInfo("Finished decompressing machine from %s to %s, in %v", machineFilePath, destination, endingTime.Sub(staringTime))
 	return nil
 }

src/catalog/providers/aws_s3_bucket/main.go

@@ -113,7 +113,7 @@ func (s *AwsS3BucketProvider) PushFile(ctx basecontext.ApiContext, rootLocalPath
 	})
 
 	// Open the file for reading.
-	file, err := os.Open(localFilePath)
+	file, err := os.Open(filepath.Clean(localFilePath))
 	if err != nil {
 		return err
 	}
@@ -150,7 +150,7 @@ func (s *AwsS3BucketProvider) PullFile(ctx basecontext.ApiContext, path string,
 	})
 
 	// Create a file to write the S3 Object contents to.
-	f, err := os.Create(destinationFilePath)
+	f, err := os.Create(filepath.Clean(destinationFilePath))
 	if err != nil {
 		return err
 	}
@@ -206,7 +206,6 @@ func (s *AwsS3BucketProvider) FileChecksum(ctx basecontext.ApiContext, path stri
 		Bucket: aws.String(s.Bucket.Name),
 		Key:    aws.String(fullPath),
 	})
-
 	if err != nil {
 		return "", err
 	}
@@ -242,7 +241,6 @@ func (s *AwsS3BucketProvider) FileExists(ctx basecontext.ApiContext, path string
 }
 
 func (s *AwsS3BucketProvider) CreateFolder(ctx basecontext.ApiContext, folderPath string, folderName string) error {
-
 	fullPath := filepath.Join(folderPath, folderName)
 	// Create a new session using the default region and credentials.
 	var err error

src/catalog/providers/azurestorageaccount/main.go

@@ -98,7 +98,7 @@ func (s *AzureStorageAccountProvider) PushFile(ctx basecontext.ApiContext, rootL
 
 	blobUrl := azblob.NewBlockBlobURL(*URL, azblob.NewPipeline(credential, azblob.PipelineOptions{}))
 
-	file, err := os.Open(localFilePath)
+	file, err := os.Open(filepath.Clean(localFilePath))
 	if err != nil {
 		return err
 	}
@@ -145,7 +145,7 @@ func (s *AzureStorageAccountProvider) PullFile(ctx basecontext.ApiContext, path
 		},
 	}))
 
-	file, err := os.Create(destinationFilePath)
+	file, err := os.Create(filepath.Clean(destinationFilePath))
 	if err != nil {
 		return err
 	}

src/catalog/providers/local/main.go

@@ -96,13 +96,13 @@ func (s *LocalProvider) PushFile(ctx basecontext.ApiContext, localRoot, path, fi
 		destPath = filepath.Join(s.Config.Path, destPath)
 	}
 
-	srcFile, err := os.Open(srcPath)
+	srcFile, err := os.Open(filepath.Clean(srcPath))
 	if err != nil {
 		return err
 	}
 	defer srcFile.Close()
 
-	destFile, err := os.Create(destPath)
+	destFile, err := os.Create(filepath.Clean(destPath))
 	if err != nil {
 		return err
 	}
@@ -123,13 +123,13 @@ func (s *LocalProvider) PullFile(ctx basecontext.ApiContext, path, filename, des
 		srcPath = filepath.Join(s.Config.Path, srcPath)
 	}
 
-	srcFile, err := os.Open(srcPath)
+	srcFile, err := os.Open(filepath.Clean(srcPath))
 	if err != nil {
 		return err
 	}
 	defer srcFile.Close()
 
-	destFile, err := os.Create(destPath)
+	destFile, err := os.Create(filepath.Clean(destPath))
 	if err != nil {
 		return err
 	}

src/cmd/api.go

@@ -37,7 +37,11 @@ func processApi(ctx basecontext.ApiContext) {
 	if err != nil {
 		panic(err)
 	}
-	os.Setenv(constants.CURRENT_USER_ENV_VAR, currentUser)
+
+	if err := os.Setenv(constants.CURRENT_USER_ENV_VAR, currentUser); err != nil {
+		panic(err)
+	}
+
 	currentUserEnv := cfg.GetKey(constants.CURRENT_USER_ENV_VAR)
 	if currentUserEnv != "" {
 		ctx.LogInfo("Running with user %s", currentUser)

src/cmd/catalog.go

@@ -37,12 +37,12 @@ func processCatalog(ctx basecontext.ApiContext) {
 
 func processCatalogHelp() {
 	fmt.Println("Usage: pd-api-service catalog <command>")
-	fmt.Println("  run\t\t\t\t\t\t\tRun a catalog file")
-	fmt.Println("  list\t\t\t\t\t\tList all catalogs")
-	fmt.Println("  push <catalog>\t\t\t\t\tPush a catalog to the server")
-	fmt.Println("  pull <catalog>\t\t\t\t\tPull a catalog from the server")
-	fmt.Println("  delete <catalog>\t\t\t\t\tDelete a catalog from the server")
-	fmt.Println("  import <catalog> <file>\t\t\t\tImport a catalog from a file")
+	fmt.Println("  run\t\t\t\t\tRun a catalog file")
+	fmt.Println("  list\t\t\t\t\tList all catalogs")
+	fmt.Println("  push <catalog>\t\t\tPush a catalog to the server")
+	fmt.Println("  pull <catalog>\t\t\tPull a catalog from the server")
+	fmt.Println("  delete <catalog>\t\t\tDelete a catalog from the server")
+	fmt.Println("  import <catalog> <file>\t\tImport a catalog from a file")
 }
 
 func catalogInitPdFile(ctx basecontext.ApiContext, cmd string) *pdfile.PDFile {

src/cmd/help.go

@@ -16,6 +16,8 @@ func processHelp(command string) {
 		processApiHelp()
 	case constants.CATALOG_COMMAND:
 		processCatalogHelp()
+	case constants.TEST_COMMAND:
+		processTestHelp()
 	default:
 		processDefaultHelp()
 	}

src/cmd/main.go

@@ -32,6 +32,10 @@ func Process() {
 	case constants.UPDATE_ROOT_PASSWORD_COMMAND:
 		processRootPassword(ctx)
 	default:
+		if helper.GetFlagSwitch("help", false) {
+			processHelp("")
+			os.Exit(0)
+		}
 		processApi(ctx)
 	}
 

src/cmd/test_providers.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/Parallels/pd-api-service/basecontext"
@@ -10,13 +11,22 @@ import (
 )
 
 func processTestProviders(ctx basecontext.ApiContext) {
-	// Checking if we just want to test
-	if helper.GetFlagSwitch(constants.TEST_CATALOG_PROVIDERS_FLAG, false) {
+	subcommand := helper.GetCommandAt(1)
+	switch subcommand {
+	case constants.TEST_CATALOG_PROVIDERS_FLAG:
 		if err := tests.TestCatalogProviders(ctx); err != nil {
 			ctx.LogError(err.Error())
 			os.Exit(1)
 		}
+	default:
+		processHelp(constants.TEST_COMMAND)
+
 	}
 
 	os.Exit(0)
 }
+
+func processTestHelp() {
+	fmt.Println("Usage: pd-api-service test <command>")
+	fmt.Println("  catalog-providers\t\t\tRun a test on all the catalog providers")
+}

src/constants/jwt.go

@@ -2,7 +2,7 @@ package constants
 
 const (
 	JWT_PRIVATE_KEY_ENV_VAR    = "JWT_PRIVATE_KEY"
-	JWT_HMACS_SECRET_ENV_VAR   = "JWT_HMACS_SECRET"
+	JWT_HMACS_SECRET_ENV_VAR   = "JWT_HMACS_SECRET" // #nosec G101 This is not a hardcoded password, it is just the variable name we use to store the secret
 	JWT_DURATION_ENV_VAR       = "JWT_DURATION"
 	JWT_SIGN_ALGORITHM_ENV_VAR = "JWT_SIGN_ALGORITHM"
 )

src/controllers/orchestrator.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"encoding/json"
+	"errors"
 	"net/http"
 	"strings"
 	"sync"
@@ -1141,7 +1142,7 @@ func CreateOrchestratorHostVirtualMachineHandler() restapi.ControllerHandler {
 		}
 
 		orchestratorSvc := orchestrator.NewOrchestratorService(ctx)
-		response, err := orchestratorSvc.CreateHostVirtualMachine(host, request)
+		response, err := orchestratorSvc.CreateHostVirtualMachine(*host, request)
 		if err != nil {
 			ReturnApiError(ctx, w, models.NewFromError(err))
 			return
@@ -1209,7 +1210,7 @@ func CreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler {
 			return
 		}
 
-		var host *data_models.OrchestratorHost
+		var hostErr error
 		var response models.CreateVirtualMachineResponse
 		var apiError *models.ApiErrorResponse
 
@@ -1237,14 +1238,13 @@ func CreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler {
 			}
 			if orchestratorHost.Resources.TotalAvailable.LogicalCpuCount > specs.GetCpuCount() &&
 				orchestratorHost.Resources.TotalAvailable.MemorySize > specs.GetMemorySize() {
-				host = &orchestratorHost
 
 				if orchestratorHost.State != "healthy" {
 					apiError = &models.ApiErrorResponse{
 						Message: "Host is not healthy",
 						Code:    400,
 					}
-					host = nil
+					hostErr = errors.New("host is not healthy")
 					continue
 				}
 
@@ -1253,24 +1253,24 @@ func CreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler {
 						Message: "Host does not have enough CPU resources",
 						Code:    400,
 					}
-					host = nil
+					hostErr = errors.New("host does not have enough CPU resources")
 					continue
 				}
 				if orchestratorHost.Resources.TotalAvailable.MemorySize < 2048 {
 					apiError = &models.ApiErrorResponse{
 						Message: "Host does not have enough Memory resources",
 						Code:    400,
 					}
-					host = nil
+					hostErr = errors.New("host does not have enough Memory resources")
 					continue
 				}
 
 				orchestratorSvc := orchestrator.NewOrchestratorService(ctx)
-				resp, err := orchestratorSvc.CreateHostVirtualMachine(host, request)
+				resp, err := orchestratorSvc.CreateHostVirtualMachine(orchestratorHost, request)
 				if err != nil {
 					e := models.NewFromError(err)
 					apiError = &e
-					host = nil
+					hostErr = err
 					break
 				} else {
 					response = *resp
@@ -1279,7 +1279,7 @@ func CreateOrchestratorVirtualMachineHandler() restapi.ControllerHandler {
 			}
 		}
 
-		if host == nil {
+		if hostErr == nil {
 			if apiError != nil {
 				ReturnApiError(ctx, w, *apiError)
 			} else {