Skip to content

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

Merged
pelikhan merged 6 commits into
mainfrom
copilot/fix-zizmor-output-issues
Oct 28, 2025
Merged

Fix zizmor output: suppress 0 warnings, show details with location, run per-file#2663
pelikhan merged 6 commits into
mainfrom
copilot/fix-zizmor-output-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 28, 2025

Zizmor security scanner output was cluttered with "0 warnings" messages, lacked diagnostic details, and ran in batch at the end of compilation instead of per-file.

Changes

  • Skip 0 warning display: Files with no findings produce no output
  • Show detailed findings: Display severity, type, location (line/column), and description for each warning
    🌈 zizmor 2 warnings in ./.github/workflows/test.lock.yml
    ✗   - [Medium] excessive-permissions at line 7, column 5: overly broad permissions
    ✗   - [High] template-injection at line 12, column 24: template injection with untrusted input
    
  • Run per-file: Execute zizmor immediately after each workflow compiles via new runZizmorOnFile() function
  • Remove batch execution: Deleted runZizmor() and end-of-compilation scanning code
  • Error formatting: Use console.FormatErrorMessage() for consistent error display with ✗ symbol

Implementation

pkg/cli/zizmor.go

  • Enhanced zizmorFinding struct to capture desc, url, annotation, and location data (start_point with row/column)
  • Modified parseAndDisplayZizmorOutput() to skip count == 0, extract location info, and display detailed findings with line/column numbers (1-based indexing) and descriptions
  • Added runZizmorOnFile() for single-file scanning with Docker volume mount to git root
  • Used console.FormatErrorMessage() for consistent error formatting

pkg/cli/compile_command.go

  • Added runZizmorPerFile and strict parameters to validation functions
  • Integrated zizmor execution into CompileWorkflowWithValidation() and CompileWorkflowDataWithValidation()
  • Removed batch execution blocks (lines 306-337, 480-485)

pkg/cli/zizmor_test.go

  • Updated test data with complete zizmor JSON structure including desc, url, and location information
  • Updated expectations: no output for 0 warnings, detailed output with location and description for findings
  • Verified error formatting with ✗ symbol prefix

pkg/cli/compile_integration_test.go

  • Updated TestCompileWithZizmor to accept silent output for clean workflows
Original prompt
  • do not show "🌈 zizmor 0 warnings" for 0 warnings.
  • run zizmor and display the result as your run workflows, not all at the end
  • display zizmor error and warnings to the console

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits October 28, 2025 07:20
- 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>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot you need to show the file, line, column and error description for zizmor errors.

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>
Copilot AI changed the title [WIP] Fix output display for zero warnings in Zizmor Fix zizmor output: suppress 0 warnings, show details, run per-file Oct 28, 2025
Copilot AI requested a review from pelikhan October 28, 2025 07:34
- 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>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 28, 2025

@copilot you need to show the file, line, column and error description for zizmor errors.

Added detailed zizmor output showing file, line, column, and error description. The output now displays:

🌈 zizmor 2 warnings in ./.github/workflows/test.lock.yml
✗   - [Medium] excessive-permissions at line 7, column 5: overly broad permissions
✗   - [High] template-injection at line 12, column 24: template injection with untrusted input

Changes in commit f88d0d4:

  • Enhanced struct to capture location (row/column) and description from zizmor JSON
  • Display shows line number, column number (1-based), and human-readable description
  • Used console.FormatErrorMessage for consistent error formatting with ✗ symbol

Copilot AI changed the title Fix zizmor output: suppress 0 warnings, show details, run per-file Fix zizmor output: suppress 0 warnings, show details with location, run per-file Oct 28, 2025
@pelikhan pelikhan marked this pull request as ready for review October 28, 2025 07:54
Copilot AI review requested due to automatic review settings October 28, 2025 07:55
@pelikhan pelikhan merged commit cf30d7f into main Oct 28, 2025
5 checks passed
@pelikhan pelikhan deleted the copilot/fix-zizmor-output-issues branch October 28, 2025 07:55
@github-actions
Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the zizmor security scanner integration to improve output clarity and execution timing. The changes suppress zero-warning messages, add detailed diagnostic information including location and severity, and execute zizmor per-file during compilation rather than in batch afterward.

Key changes:

  • Suppresses "0 warnings" output for clean workflows
  • Displays detailed findings with severity, type, line/column location, and descriptions
  • Executes zizmor per-file during compilation instead of batch execution at the end

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/cli/zizmor.go Enhanced zizmorFinding struct with location/description fields; refactored to runZizmorOnFile() for single-file scanning; updated output formatting to skip zero warnings and show detailed diagnostics
pkg/cli/compile_command.go Integrated per-file zizmor execution into validation functions; removed batch execution code blocks
pkg/cli/zizmor_test.go Updated test fixtures with complete zizmor JSON structure and verified new output format
pkg/cli/compile_integration_test.go Updated test to accept silent output for workflows with no findings
pkg/cli/add_command.go Updated function calls to match new signature with zizmor parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/cli/zizmor.go
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 {
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition row > 0 || col > 0 will incorrectly omit location info when both row and column are 0, which is a valid location (first line, first column). The condition should be row >= 0 && col >= 0 or simply always format the location since negative values should not occur in valid zizmor output.

Suggested change
if row > 0 || col > 0 {
if row >= 0 && col >= 0 {

Copilot uses AI. Check for mistakes.

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 {
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The expression zizmor && !noEmit is duplicated in multiple calls (lines 252, 393). Consider extracting this into a named boolean variable like runZizmor := zizmor && !noEmit at the function start to improve readability and reduce duplication.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants