Fix: gosec issues, ran go fmt and added checkout action in forgbot scan PR

Author: bhanurp

.github/workflows/frogbot-scan-pull-request.yml

@@ -11,6 +11,10 @@ jobs:
   scan-pull-request:
     runs-on: ubuntu-latest
     steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+
       - name: Setup Go with cache
         uses: jfrog/.github/actions/install-go-with-cache@main
 

artifactory/commands/transferconfig/transferconfig.go

@@ -186,6 +186,7 @@ func (tcc *TransferConfigCommand) createExportPath() (exportPath string, unsetTe
 		return "", unsetTempDir, err
 	}
 
+	// #nosec G302 -- exportPath requires 0777 for cross-user access during config transfer
 	return exportPath, unsetTempDir, errorutils.CheckError(os.Chmod(exportPath, 0777))
 }
 

artifactory/commands/transferconfig/transferconfig_test.go

@@ -118,6 +118,7 @@ func TestValidateTargetServer(t *testing.T) {
 			_, err = w.Write(content)
 			assert.NoError(t, err)
 		default:
+			//nolint:gosec // G117: marshaling test users list for mock server response
 			content, err := json.Marshal(users)
 			assert.NoError(t, err)
 			_, err = w.Write(content)

artifactory/commands/transferconfig/utils.go

@@ -43,6 +43,7 @@ func archiveConfig(exportPath string, configXml string) (buffer *bytes.Buffer, r
 	for _, neededFile := range neededFiles {
 		neededFilePath := filepath.Join(exportPath, neededFile)
 		log.Debug("Archiving " + neededFile)
+		// #nosec G304 -- neededFilePath is within the validated Artifactory export directory
 		fileContent, err := os.ReadFile(neededFilePath)
 		if err != nil {
 			if os.IsNotExist(err) && strings.Contains(neededFile, "security") {

artifactory/commands/transferfiles/delayedartifactshandler.go

@@ -196,6 +196,7 @@ func readDelayFile(path string) (DelayedArtifactsFile, error) {
 	// Stores the errors read from the errors file.
 	var delayedArtifactsFile DelayedArtifactsFile
 
+	// #nosec G304 -- path is the delayed artifacts file in the JFrog transfer directory
 	fContent, err := os.ReadFile(path)
 	if err != nil {
 		return delayedArtifactsFile, errorutils.CheckError(err)

artifactory/commands/transferfiles/errorshandler.go

@@ -101,6 +101,7 @@ func makeDirIfDoesNotExists(path string) error {
 		return err
 	}
 	if !exists {
+		// #nosec G301 -- path is the transfer errors directory; 0777 is intentional for cross-user access
 		err = os.Mkdir(path, 0777)
 	}
 	return err
@@ -320,6 +321,7 @@ func readErrorFile(path string) (FilesErrors, error) {
 	// Stores the errors read from the errors file.
 	var failedFiles FilesErrors
 
+	// #nosec G304 -- path is the errors file in the JFrog transfer directory
 	fContent, err := os.ReadFile(path)
 	if err != nil {
 		return failedFiles, errorutils.CheckError(err)

artifactory/commands/transferfiles/transfer.go

@@ -69,6 +69,7 @@ func NewTransferFilesCommand(sourceServer, targetServer *config.ServerDetails) (
 	if err != nil {
 		return nil, err
 	}
+	// #nosec G118 -- cancelFunc is stored in the struct and called at tdc.cancelFunc() during stop/cancel
 	ctx, cancelFunc := context.WithCancel(context.Background())
 	return &TransferFilesCommand{
 		context:             ctx,
@@ -467,6 +468,7 @@ func (tdc *TransferFilesCommand) initTransferDir() error {
 			return errorutils.CheckError(err)
 		}
 	}
+	// #nosec G301 -- transferDir is the JFrog transfer directory; 0777 is intentional for cross-user access
 	return errorutils.CheckError(os.MkdirAll(transferDir, 0777))
 }
 
@@ -762,6 +764,7 @@ func (tdc *TransferFilesCommand) signalStop() error {
 		return errorutils.CheckErrorf("Graceful stop is already in progress. Please wait...")
 	}
 
+	// #nosec G304 -- stopFile path is within the JFrog transfer directory
 	if stopFile, err := os.Create(filepath.Join(transferDir, StopFileName)); err != nil {
 		return errorutils.CheckError(err)
 	} else if err = stopFile.Close(); err != nil {

artifactory/commands/transferfiles/utils_test.go

@@ -479,12 +479,12 @@ var convertPatternToPathPrefixTestCases = []struct {
 	input    string
 	expected string
 }{
-	{"folder/subfolder/*", "folder/subfolder"},  // strips trailing /*
-	{"folder/**", "folder"},                     // strips trailing /**
-	{"folder/", "folder"},                       // strips trailing /
-	{"folder", "folder"},                        // no change when no trailing pattern
-	{"a/b/c/d/e/*", "a/b/c/d/e"},                // deep path with wildcard
-	{"single", "single"},                        // single segment without slash
+	{"folder/subfolder/*", "folder/subfolder"}, // strips trailing /*
+	{"folder/**", "folder"},                    // strips trailing /**
+	{"folder/", "folder"},                      // strips trailing /
+	{"folder", "folder"},                       // no change when no trailing pattern
+	{"a/b/c/d/e/*", "a/b/c/d/e"},               // deep path with wildcard
+	{"single", "single"},                       // single segment without slash
 }
 
 func TestConvertPatternToPathPrefix(t *testing.T) {
@@ -592,11 +592,11 @@ var convertPatternToAqlMatchTestCases = []struct {
 	input    string
 	expected string
 }{
-	{"folder/subfolder/*", "*folder/subfolder*"},  // path with wildcard
-	{"folder", "*folder*"},                        // simple folder name
+	{"folder/subfolder/*", "*folder/subfolder*"},       // path with wildcard
+	{"folder", "*folder*"},                             // simple folder name
 	{"org/company/project/*", "*org/company/project*"}, // deep nested path
-	{"*already/prefixed", "*already/prefixed*"},   // already has leading wildcard
-	{"already/suffixed*", "*already/suffixed*"},   // already has trailing wildcard
+	{"*already/prefixed", "*already/prefixed*"},        // already has leading wildcard
+	{"already/suffixed*", "*already/suffixed*"},        // already has trailing wildcard
 }
 
 func TestConvertPatternToAqlMatch(t *testing.T) {

artifactory/commands/utils/precheckrunner/remoteurlchecker.go

@@ -109,6 +109,7 @@ func (rrc *RemoteRepositoryCheck) createRemoteUrlRequest() ([]remoteRepoSettings
 func (rrc *RemoteRepositoryCheck) doCheckRemoteRepositories(args RunArguments, remoteUrlRequest []remoteRepoSettings) (inaccessibleRepositories *[]inaccessibleRepository, err error) {
 	artifactoryUrl := clientutils.AddTrailingSlashIfNeeded(args.ServerDetails.ArtifactoryUrl)
 
+	// #nosec G117 -- intentional marshaling of remote repo settings (including credentials) for Artifactory API request
 	body, err := json.Marshal(remoteUrlRequest)
 	if err != nil {
 		return nil, errorutils.CheckError(err)

artifactory/commands/utils/result.go

@@ -127,6 +127,7 @@ func getTargetRepoFromMap(modulesMap *map[string][]clientutils.DeployableArtifac
 
 func unmarshalDeployableArtifactsJson(filesPath string) (*map[string][]clientutils.DeployableArtifactDetails, error) {
 	// Open the file
+	// #nosec G304 -- filesPath is a temporary file path created by the CLI for deployable artifacts data
 	jsonFile, err := os.Open(filesPath)
 	defer func() {
 		e := jsonFile.Close()