Fix zizmor output: suppress 0 warnings, show details with location, run per-file (#2663)

* Initial plan

* Fix zizmor output: skip 0 warnings, show details, run per-file

- Skip displaying "🌈 zizmor 0 warnings" for files with 0 warnings
- Display detailed findings (severity and type) for each warning
- Run zizmor per-file as each workflow compiles instead of all at the end
- Update tests to match new behavior

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Run go fmt

* Remove unused runZizmor function and console import

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Fix integration test for new zizmor behavior

Updated TestCompileWithZizmor to handle the new behavior where
workflows with 0 warnings don't display any zizmor output.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Add detailed zizmor output with file, line, column, and description

- Updated zizmorFinding struct to capture desc, url, annotation, and location info
- Modified display logic to show line number, column number, and description
- Used console.FormatErrorMessage for consistent error formatting
- Updated all tests with enhanced JSON structure and expected output
- Line/column numbers are displayed in 1-based indexing for user readability

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Author: Copilot

pkg/cli/add_command.go

@@ -551,7 +551,7 @@ func updateWorkflowTitle(content string, number int) string {
 func compileWorkflow(filePath string, verbose bool, engineOverride string) error {
 	// Create compiler and compile the workflow
 	compiler := workflow.NewCompiler(verbose, engineOverride, GetVersion())
-	if err := CompileWorkflowWithValidation(compiler, filePath, verbose); err != nil {
+	if err := CompileWorkflowWithValidation(compiler, filePath, verbose, false, false); err != nil {
 		return err
 	}
 
@@ -607,7 +607,7 @@ func compileWorkflowWithTracking(filePath string, verbose bool, engineOverride s
 	// Create compiler and set the file tracker
 	compiler := workflow.NewCompiler(verbose, engineOverride, GetVersion())
 	compiler.SetFileTracker(tracker)
-	if err := CompileWorkflowWithValidation(compiler, filePath, verbose); err != nil {
+	if err := CompileWorkflowWithValidation(compiler, filePath, verbose, false, false); err != nil {
 		return err
 	}
 

pkg/cli/compile_command.go

@@ -20,7 +20,7 @@ import (
 var compileLog = logger.New("cli:compile_command")
 
 // CompileWorkflowWithValidation compiles a workflow with always-on YAML validation for CLI usage
-func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string, verbose bool) error {
+func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string, verbose bool, runZizmorPerFile bool, strict bool) error {
 	// Compile the workflow first
 	if err := compiler.CompileWorkflow(filePath); err != nil {
 		return err
@@ -46,12 +46,19 @@ func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string,
 		return fmt.Errorf("generated lock file is not valid YAML: %w", err)
 	}
 
+	// Run zizmor on the generated lock file if requested
+	if runZizmorPerFile {
+		if err := runZizmorOnFile(lockFile, verbose, strict); err != nil {
+			return fmt.Errorf("zizmor security scan failed: %w", err)
+		}
+	}
+
 	return nil
 }
 
 // CompileWorkflowDataWithValidation compiles from already-parsed WorkflowData with validation
 // This avoids re-parsing when the workflow data has already been parsed
-func CompileWorkflowDataWithValidation(compiler *workflow.Compiler, workflowData *workflow.WorkflowData, filePath string, verbose bool) error {
+func CompileWorkflowDataWithValidation(compiler *workflow.Compiler, workflowData *workflow.WorkflowData, filePath string, verbose bool, runZizmorPerFile bool, strict bool) error {
 	// Compile the workflow using already-parsed data
 	if err := compiler.CompileWorkflowData(workflowData, filePath); err != nil {
 		return err
@@ -77,6 +84,13 @@ func CompileWorkflowDataWithValidation(compiler *workflow.Compiler, workflowData
 		return fmt.Errorf("generated lock file is not valid YAML: %w", err)
 	}
 
+	// Run zizmor on the generated lock file if requested
+	if runZizmorPerFile {
+		if err := runZizmorOnFile(lockFile, verbose, strict); err != nil {
+			return fmt.Errorf("zizmor security scan failed: %w", err)
+		}
+	}
+
 	return nil
 }
 
@@ -235,7 +249,7 @@ func CompileWorkflows(config CompileConfig) ([]*workflow.WorkflowData, error) {
 			workflowDataList = append(workflowDataList, workflowData)
 
 			compileLog.Printf("Starting compilation of %s", resolvedFile)
-			if err := CompileWorkflowDataWithValidation(compiler, workflowData, resolvedFile, verbose); err != nil {
+			if err := CompileWorkflowDataWithValidation(compiler, workflowData, resolvedFile, verbose, zizmor && !noEmit, strict); err != nil {
 				// Always put error on a new line and don't wrap with "failed to compile workflow"
 				fmt.Fprintln(os.Stderr, err.Error())
 				errorMessages = append(errorMessages, err.Error())
@@ -303,25 +317,6 @@ func CompileWorkflows(config CompileConfig) ([]*workflow.WorkflowData, error) {
 			return workflowDataList, fmt.Errorf("compilation failed")
 		}
 
-		// Run zizmor security scanner if requested and compilation was successful
-		if zizmor && !noEmit {
-			// Resolve workflow directory path
-			if workflowDir == "" {
-				workflowDir = ".github/workflows"
-			}
-			absWorkflowDir := workflowDir
-			if !filepath.IsAbs(absWorkflowDir) {
-				gitRoot, err := findGitRoot()
-				if err == nil {
-					absWorkflowDir = filepath.Join(gitRoot, workflowDir)
-				}
-			}
-
-			if err := runZizmor(absWorkflowDir, verbose, strict); err != nil {
-				return workflowDataList, fmt.Errorf("zizmor security scan failed: %w", err)
-			}
-		}
-
 		return workflowDataList, nil
 	}
 
@@ -395,7 +390,7 @@ func CompileWorkflows(config CompileConfig) ([]*workflow.WorkflowData, error) {
 		}
 		workflowDataList = append(workflowDataList, workflowData)
 
-		if err := CompileWorkflowDataWithValidation(compiler, workflowData, file, verbose); err != nil {
+		if err := CompileWorkflowDataWithValidation(compiler, workflowData, file, verbose, zizmor && !noEmit, strict); err != nil {
 			// Print the error to stderr (errors from CompileWorkflow are already formatted)
 			fmt.Fprintln(os.Stderr, err.Error())
 			errorCount++
@@ -482,13 +477,6 @@ func CompileWorkflows(config CompileConfig) ([]*workflow.WorkflowData, error) {
 		return workflowDataList, fmt.Errorf("compilation failed")
 	}
 
-	// Run zizmor security scanner if requested and compilation was successful
-	if zizmor && !noEmit {
-		if err := runZizmor(workflowsDir, verbose, strict); err != nil {
-			return workflowDataList, fmt.Errorf("zizmor security scan failed: %w", err)
-		}
-	}
-
 	return workflowDataList, nil
 }
 
@@ -590,7 +578,7 @@ func watchAndCompileWorkflows(markdownFile string, compiler *workflow.Compiler,
 		if verbose {
 			fmt.Fprintf(os.Stderr, "🔨 Initial compilation of %s...\n", markdownFile)
 		}
-		if err := CompileWorkflowWithValidation(compiler, markdownFile, verbose); err != nil {
+		if err := CompileWorkflowWithValidation(compiler, markdownFile, verbose, false, false); err != nil {
 			// Always show initial compilation errors on new line without wrapping
 			fmt.Fprintln(os.Stderr, err.Error())
 			stats.Errors++
@@ -702,7 +690,7 @@ func compileAllWorkflowFiles(compiler *workflow.Compiler, workflowsDir string, v
 		if verbose {
 			fmt.Printf("🔨 Compiling: %s\n", file)
 		}
-		if err := CompileWorkflowWithValidation(compiler, file, verbose); err != nil {
+		if err := CompileWorkflowWithValidation(compiler, file, verbose, false, false); err != nil {
 			// Always show compilation errors on new line
 			fmt.Fprintln(os.Stderr, err.Error())
 			stats.Errors++
@@ -759,7 +747,7 @@ func compileModifiedFiles(compiler *workflow.Compiler, files []string, verbose b
 			fmt.Fprintf(os.Stderr, "🔨 Compiling: %s\n", file)
 		}
 
-		if err := CompileWorkflowWithValidation(compiler, file, verbose); err != nil {
+		if err := CompileWorkflowWithValidation(compiler, file, verbose, false, false); err != nil {
 			// Always show compilation errors on new line
 			fmt.Fprintln(os.Stderr, err.Error())
 			stats.Errors++

pkg/cli/compile_integration_test.go

@@ -324,26 +324,16 @@ This workflow tests the zizmor security scanner integration.
 
 	outputStr := string(output)
 
-	// Check that zizmor was run
-	if !strings.Contains(outputStr, "zizmor") && !strings.Contains(outputStr, "Zizmor") {
-		t.Errorf("Output should mention zizmor scanner")
-	}
-
-	// Check for the new formatted output: "🌈 zizmor X warnings in <filepath>"
-	if !strings.Contains(outputStr, "🌈 zizmor") {
-		t.Errorf("Output should contain formatted zizmor message with rainbow emoji")
-	}
-
-	// Verify the format includes "warnings in" or "warning in"
-	hasWarningsFormat := strings.Contains(outputStr, "warnings in") || strings.Contains(outputStr, "warning in")
-	if !hasWarningsFormat {
-		t.Errorf("Output should contain 'warnings in' or 'warning in' format")
-	}
+	// Note: With the new behavior, if there are 0 warnings, no zizmor output is displayed
+	// The test just verifies that the command succeeds with --zizmor flag
+	// If there are warnings, they will be shown in the format:
+	// "🌈 zizmor X warnings in <filepath>"
+	//   - [Severity] finding-type
 
 	// The lock file should still exist after zizmor scan
 	if _, err := os.Stat(lockFilePath); os.IsNotExist(err) {
 		t.Fatalf("Lock file was removed after zizmor scan")
 	}
 
-	t.Logf("Integration test passed - zizmor flag works correctly with new format")
+	t.Logf("Integration test passed - zizmor flag works correctly\nOutput: %s", outputStr)
 }

pkg/cli/zizmor.go

@@ -16,6 +16,8 @@ import (
 // zizmorFinding represents a single finding from zizmor JSON output
 type zizmorFinding struct {
 	Ident          string `json:"ident"`
+	Desc           string `json:"desc"`
+	URL            string `json:"url"`
 	Determinations struct {
 		Severity string `json:"severity"`
 	} `json:"determinations"`
@@ -26,36 +28,37 @@ type zizmorFinding struct {
 					GivenPath string `json:"given_path"`
 				} `json:"Local"`
 			} `json:"key"`
+			Annotation string `json:"annotation"`
 		} `json:"symbolic"`
+		Concrete struct {
+			Location struct {
+				StartPoint struct {
+					Row    int `json:"row"`
+					Column int `json:"column"`
+				} `json:"start_point"`
+			} `json:"location"`
+		} `json:"concrete"`
 	} `json:"locations"`
 }
 
-// runZizmor runs the zizmor security scanner on generated .lock.yml files using Docker
-func runZizmor(workflowsDir string, verbose bool, strict bool) error {
-	compileLog.Print("Running zizmor security scanner")
-
-	if verbose {
-		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running zizmor security scanner on generated .lock.yml files..."))
-	}
+// runZizmorOnFile runs the zizmor security scanner on a single .lock.yml file using Docker
+func runZizmorOnFile(lockFile string, verbose bool, strict bool) error {
+	compileLog.Printf("Running zizmor security scanner on %s", lockFile)
 
 	// Find git root to get the absolute path for Docker volume mount
 	gitRoot, err := findGitRoot()
 	if err != nil {
 		return fmt.Errorf("failed to find git root: %w", err)
 	}
 
-	// Get the absolute path of the workflows directory
-	var absWorkflowsDir string
-	if filepath.IsAbs(workflowsDir) {
-		absWorkflowsDir = workflowsDir
-	} else {
-		absWorkflowsDir = filepath.Join(gitRoot, workflowsDir)
+	// Get the relative path from git root
+	relPath, err := filepath.Rel(gitRoot, lockFile)
+	if err != nil {
+		return fmt.Errorf("failed to get relative path: %w", err)
 	}
 
-	compileLog.Printf("Running zizmor on directory: %s", absWorkflowsDir)
-
 	// Build the Docker command with JSON output for easier parsing
-	// docker run --rm -v "$(pwd)":/workdir -w /workdir ghcr.io/zizmorcore/zizmor:latest --format json .
+	// docker run --rm -v "$(pwd)":/workdir -w /workdir ghcr.io/zizmorcore/zizmor:latest --format json <file>
 	cmd := exec.Command(
 		"docker",
 		"run",
@@ -64,7 +67,7 @@ func runZizmor(workflowsDir string, verbose bool, strict bool) error {
 		"-w", "/workdir",
 		"ghcr.io/zizmorcore/zizmor:latest",
 		"--format", "json",
-		".",
+		relPath,
 	)
 
 	// Capture output
@@ -102,20 +105,16 @@ func runZizmor(workflowsDir string, verbose bool, strict bool) error {
 			if exitCode >= 10 && exitCode <= 14 {
 				// In strict mode, findings are treated as errors
 				if strict {
-					return fmt.Errorf("strict mode: zizmor found %d security warnings/errors - workflows must have no zizmor findings in strict mode", totalWarnings)
+					return fmt.Errorf("strict mode: zizmor found %d security warnings/errors in %s - workflows must have no zizmor findings in strict mode", totalWarnings, filepath.Base(lockFile))
 				}
 				// In non-strict mode, findings are logged but not treated as errors
 				return nil
 			}
 			// Other exit codes are actual errors
-			return fmt.Errorf("zizmor failed with exit code %d", exitCode)
+			return fmt.Errorf("zizmor failed with exit code %d on %s", exitCode, filepath.Base(lockFile))
 		}
 		// Non-ExitError errors (e.g., command not found)
-		return fmt.Errorf("zizmor failed: %w", err)
-	}
-
-	if verbose {
-		fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Zizmor security scan completed - no findings"))
+		return fmt.Errorf("zizmor failed on %s: %w", filepath.Base(lockFile), err)
 	}
 
 	return nil
@@ -124,8 +123,8 @@ func runZizmor(workflowsDir string, verbose bool, strict bool) error {
 // parseAndDisplayZizmorOutput parses zizmor JSON output and displays it in the desired format
 // Returns the total number of warnings found
 func parseAndDisplayZizmorOutput(stdout, stderr string, verbose bool) (int, error) {
-	// Count findings per file
-	fileFindings := make(map[string]int)
+	// Map findings to files for detailed display
+	fileFindings := make(map[string][]zizmorFinding)
 
 	// Parse stderr for "completed" messages to get list of files
 	completedFiles := []string{}
@@ -138,8 +137,10 @@ func parseAndDisplayZizmorOutput(stdout, stderr string, verbose bool) (int, erro
 			if len(parts) == 2 {
 				filePath := strings.TrimSpace(parts[1])
 				completedFiles = append(completedFiles, filePath)
-				// Initialize count to 0
-				fileFindings[filePath] = 0
+				// Initialize empty findings slice
+				if _, exists := fileFindings[filePath]; !exists {
+					fileFindings[filePath] = []zizmorFinding{}
+				}
 			}
 		}
 	}
@@ -152,15 +153,15 @@ func parseAndDisplayZizmorOutput(stdout, stderr string, verbose bool) (int, erro
 			return 0, fmt.Errorf("failed to parse zizmor JSON output: %w", err)
 		}
 
-		// Count findings per file - each finding counts as 1 regardless of how many locations it has
+		// Organize findings by file
 		for _, finding := range findings {
-			// Track which files this finding affects
+			// Track which files this finding affects (avoid duplicates)
 			affectedFiles := make(map[string]bool)
 			for _, location := range finding.Locations {
 				filePath := location.Symbolic.Key.Local.GivenPath
 				if filePath != "" && !affectedFiles[filePath] {
 					affectedFiles[filePath] = true
-					fileFindings[filePath]++
+					fileFindings[filePath] = append(fileFindings[filePath], finding)
 					totalWarnings++
 				}
 			}
@@ -169,13 +170,42 @@ func parseAndDisplayZizmorOutput(stdout, stderr string, verbose bool) (int, erro
 
 	// Display reformatted output for each completed file
 	for _, filePath := range completedFiles {
-		count := fileFindings[filePath]
+		findings := fileFindings[filePath]
+		count := len(findings)
+
+		// Skip files with 0 warnings
+		if count == 0 {
+			continue
+		}
+
 		// Format: 🌈 zizmor xx warnings in <filepath>
 		warningText := "warnings"
 		if count == 1 {
 			warningText = "warning"
 		}
 		fmt.Fprintf(os.Stderr, "🌈 zizmor %d %s in %s\n", count, warningText, filePath)
+
+		// Display detailed findings
+		for _, finding := range findings {
+			severity := finding.Determinations.Severity
+			ident := finding.Ident
+			desc := finding.Desc
+
+			// Find the primary location (first location in the list)
+			var locationInfo string
+			if len(finding.Locations) > 0 {
+				loc := finding.Locations[0]
+				row := loc.Concrete.Location.StartPoint.Row
+				col := loc.Concrete.Location.StartPoint.Column
+				// Zizmor uses 0-based indexing, convert to 1-based for user display
+				if row > 0 || col > 0 {
+					locationInfo = fmt.Sprintf(" at line %d, column %d", row+1, col+1)
+				}
+			}
+
+			errorMsg := fmt.Sprintf("  - [%s] %s%s: %s", severity, ident, locationInfo, desc)
+			fmt.Fprintln(os.Stderr, console.FormatErrorMessage(errorMsg))
+		}
 	}
 
 	return totalWarnings, nil