Merge pull request #1363 from LeanerCloud/fix/ci-security-gosec

fix(ci): self-install pinned gosec and resolve findings to green Security Scanning

Author: cristim

.github/workflows/ci.yml

@@ -318,11 +318,35 @@ jobs:
           fi
 
       - name: Run gosec Security Scanner
-        uses: securego/gosec@4a3bd8af174872c778439083ded7adbf3747e770 # v2.26.1
-        with:
-          args: '-fmt sarif -out gosec-results.sarif ./...'
+        run: |
+          # Install pinned gosec using the job's existing setup-go (GO_VERSION 1.26.5).
+          # The securego/gosec Docker action bundles its own Go toolchain (1.26.1) which
+          # cannot satisfy the "go 1.26.5" module requirement in go.mod, causing it to
+          # load 0 files and exit 1 with a toolchain mismatch rather than real findings.
+          go install github.com/securego/gosec/v2/cmd/gosec@v2.26.1
+          # Multi-module repo: each ./... only walks the current module so scanning root
+          # alone silently misses pkg/ and providers/*. Mirror the govulncheck per-module
+          # loop, collect per-module SARIF, then merge for the upload step.
+          set -e
+          for mod in . pkg providers/aws providers/azure providers/gcp tests/e2e; do
+            tag=$(echo "$mod" | tr './' '--' | sed 's/^-/root/')
+            out="$RUNNER_TEMP/gosec-${tag}.sarif"
+            echo "==> gosec in $mod"
+            (cd "$mod" && gosec -fmt sarif -out "$out" ./...)
+            # Code scanning rejects a SARIF file whose runs share a category
+            # (github.blog changelog 2025-07-21), so give each module's run a
+            # unique automationDetails.id before merging.
+            jq --arg id "gosec-${tag}/" '.runs |= map(.automationDetails = {id: $id})' \
+              "$out" > "$out.tmp" && mv "$out.tmp" "$out"
+          done
+          # Merge per-module SARIF runs into one file for the upload step.
+          # jq is preinstalled on the GitHub Ubuntu runner image (no new deps).
+          jq -s '{version: "2.1.0", "$schema": "https://json.schemastore.org/sarif-2.1.0.json", runs: [.[].runs[]]}' \
+            "$RUNNER_TEMP"/gosec-*.sarif > gosec-results.sarif
+          echo "Merged $(jq '.runs | length' gosec-results.sarif) SARIF runs"
 
       - name: Upload gosec results to GitHub Security
+        if: always()
         uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
         with:
           sarif_file: gosec-results.sarif
@@ -335,17 +359,48 @@ jobs:
           format: 'sarif'
           output: 'trivy-results.sarif'
           severity: 'CRITICAL,HIGH'
+          # Pin the Trivy binary independently of the action SHA. v0.36.0 is
+          # the latest trivy-action release but bundles Trivy v0.70.0, which
+          # panics in adaptDefaultTags on terraform/environments/aws/main.tf
+          # (null default_tags vars). Fixed in Trivy >= v0.72.0. The action
+          # forwards this input to aquasecurity/setup-trivy, so both scan
+          # steps run the same pinned binary. Keep both steps on this version.
+          version: 'v0.72.0'
 
       - name: Upload Trivy results to GitHub Security
         uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
         with:
           sarif_file: 'trivy-results.sarif'
 
-      - name: Run tfsec (Terraform Security)
-        uses: aquasecurity/tfsec-action@b466648d6e39e7c75324f25d83891162a721f2d6 # v1.0.3
+      # Terraform IaC misconfiguration scanning. Replaces the deprecated
+      # aquasecurity/tfsec-action, whose bundled HCL parser rejects Terraform
+      # 1.5+ `check {}` blocks (e.g. terraform/modules/deployment-checks/main.tf)
+      # with a hard "scan failed" parse error that soft_fail does not suppress
+      # (soft_fail only downgrades findings, not scan errors). Trivy is tfsec's
+      # official successor, parses `check {}` blocks, and (like the fs scan
+      # above) uses the default exit-code 0 so misconfig findings are reported
+      # to the Security tab without gating the job -- matching tfsec's prior
+      # soft_fail: true behaviour while preserving Terraform IaC coverage.
+      - name: Run Trivy IaC misconfiguration scanner (Terraform)
+        uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0
+        with:
+          scan-type: 'config'
+          scan-ref: 'terraform/'
+          format: 'sarif'
+          output: 'trivy-config-results.sarif'
+          severity: 'CRITICAL,HIGH'
+          # Same pinned Trivy binary as the filesystem scan above (>= v0.72.0
+          # avoids the adaptDefaultTags panic on null default_tags vars).
+          version: 'v0.72.0'
+
+      - name: Upload Trivy IaC results to GitHub Security
+        if: always()
+        uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0
         with:
-          working_directory: terraform/
-          soft_fail: true
+          sarif_file: 'trivy-config-results.sarif'
+          # Distinct category so this IaC analysis does not overwrite the
+          # filesystem Trivy analysis uploaded above (both report as "Trivy").
+          category: 'trivy-iac'
 
   # Snyk security scanning
   snyk-scan:

cmd/configure_azure.go

@@ -130,7 +130,7 @@ func storeAzureCredentials(ctx context.Context, store SecretsStore, stackName st
 	}
 
 	// Marshal credentials to JSON
-	credJSON, err := json.Marshal(creds)
+	credJSON, err := json.Marshal(creds) // #nosec G117 -- intentional: marshaling Azure credential struct (contains ClientSecret field) for secure storage in the credential store
 	if err != nil {
 		return fmt.Errorf("failed to marshal credentials: %w", err)
 	}
@@ -341,7 +341,7 @@ func createAzureServicePrincipal(reader *bufio.Reader, subscriptionID string) er
 	if choice == "r" || choice == "run" || choice == "" {
 		fmt.Println()
 		fmt.Println(strings.Repeat("-", 60))
-		cmd := exec.Command("az", "ad", "sp", "create-for-rbac",
+		cmd := exec.Command("az", "ad", "sp", "create-for-rbac", // #nosec G204 -- binary "az" is hardcoded; subscriptionID validated by validateAzureUUID before exec
 			"--name", "CUDly",
 			"--role", "Reservations Administrator",
 			"--scopes", fmt.Sprintf("/subscriptions/%s", subscriptionID))
@@ -401,7 +401,7 @@ func executeExplicitCommand(reader *bufio.Reader, displayCmd string, program str
 	fmt.Printf("Executing: %s\n", displayCmd)
 	fmt.Println(strings.Repeat("-", 60))
 
-	cmd := exec.Command(program, args...)
+	cmd := exec.Command(program, args...) // #nosec G204 -- configure CLI tool; program is always "az" (Azure CLI) per all callers; no user input reaches this function
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	cmd.Stdin = os.Stdin

cmd/configure_gcp.go

@@ -210,7 +210,7 @@ func loadAWSConfigForGCP(ctx context.Context) (aws.Config, error) {
 func loadAndUpdateGCPCredentials(credsFile string) (GCPCredentials, []byte, error) {
 	expandedPath := expandHomeDirectory(credsFile)
 
-	credsData, err := os.ReadFile(expandedPath)
+	credsData, err := os.ReadFile(expandedPath) // #nosec G304 -- GCP credentials file path is operator-supplied via CLI argument; operator controls the value
 	if err != nil {
 		return GCPCredentials{}, nil, fmt.Errorf("failed to read credentials file: %w", err)
 	}
@@ -222,7 +222,7 @@ func loadAndUpdateGCPCredentials(credsFile string) (GCPCredentials, []byte, erro
 
 	if gcpOpts.ProjectID != "" {
 		creds.ProjectID = gcpOpts.ProjectID
-		credsData, err = json.Marshal(creds)
+		credsData, err = json.Marshal(creds) // #nosec G117 -- intentional: marshaling GCP credential struct (contains PrivateKey field) for secure storage in the credential store
 		if err != nil {
 			return GCPCredentials{}, nil, fmt.Errorf("failed to marshal updated credentials: %w", err)
 		}
@@ -289,7 +289,7 @@ func runGCPSetupCommands(reader *bufio.Reader) (string, error) {
 	// Set the project - use exec.Command with arguments instead of shell
 	fmt.Println()
 	fmt.Println("Setting project...")
-	cmd := exec.Command("gcloud", "config", "set", "project", projectID)
+	cmd := exec.Command("gcloud", "config", "set", "project", projectID) // #nosec G204 -- binary "gcloud" is hardcoded; projectID validated by validateGCPProjectID before exec
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	if err := cmd.Run(); err != nil {
@@ -407,7 +407,7 @@ func executeGCPCommand(reader *bufio.Reader, displayCmd string, program string,
 	fmt.Printf("Executing: %s\n", displayCmd)
 	fmt.Println(strings.Repeat("-", 60))
 
-	cmd := exec.Command(program, args...)
+	cmd := exec.Command(program, args...) // #nosec G204 -- configure CLI tool; program is always "gcloud" per all callers; no user input reaches this function
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	cmd.Stdin = os.Stdin

cmd/helpers.go

@@ -533,7 +533,7 @@ func ConfirmPurchase(totalInstances int, totalSavings float64, skipConfirmation
 // CheckAuditLogWritable opens the audit log file in append mode to verify it is writable.
 // Returns an error if the path cannot be opened for writing.
 func CheckAuditLogWritable(path string) error {
-	f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
+	f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) // #nosec G304 -- audit log path is operator-configured; value is not reachable from user input
 	if err != nil {
 		return fmt.Errorf("audit log %q not writable: %w", path, err)
 	}

cmd/multi_service_csv.go

@@ -27,7 +27,7 @@ func determineCSVCoverage(cfg Config) float64 {
 
 // loadRecommendationsFromCSV reads and returns recommendations from a CSV file.
 func loadRecommendationsFromCSV(csvPath string) ([]common.Recommendation, error) {
-	file, err := os.Open(csvPath)
+	file, err := os.Open(csvPath) // #nosec G304 -- CLI tool: csvPath is an operator-supplied command-line argument
 	if err != nil {
 		return nil, fmt.Errorf("failed to open CSV file: %w", err)
 	}
@@ -195,7 +195,7 @@ func writeMultiServiceCSVReport(results []common.PurchaseResult, filepath string
 		return nil
 	}
 
-	file, err := os.Create(filepath)
+	file, err := os.Create(filepath) // #nosec G304 -- CLI tool: filepath is an operator-supplied output path argument
 	if err != nil {
 		return fmt.Errorf("failed to create CSV file: %w", err)
 	}