From bce261f4eb89758b154da9de50168d818e21ed4a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 04:29:43 +0000 Subject: [PATCH 1/3] Initial plan From e56b5e879bae96cb4aa51cb5d28894948f03ba72 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 04:56:26 +0000 Subject: [PATCH 2/3] Fix static analysis pipeline execution gap and output assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../workflows/static-analysis-report.lock.yml | 6 +- .github/workflows/static-analysis-report.md | 20 + pkg/cli/actionlint.go | 1 + pkg/cli/compile_external_tools_test.go | 38 ++ pkg/cli/compile_pipeline.go | 436 +++++++++--------- pkg/cli/grype.go | 4 +- pkg/cli/shellcheck.go | 132 +++--- pkg/cli/syft.go | 4 +- 8 files changed, 349 insertions(+), 292 deletions(-) diff --git a/.github/workflows/static-analysis-report.lock.yml b/.github/workflows/static-analysis-report.lock.yml index adbedfb0c21..1ba27ae2565 100644 --- a/.github/workflows/static-analysis-report.lock.yml +++ b/.github/workflows/static-analysis-report.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"65ffe58b3e57275e946812d88fb3e1b79d9ffd9d9543e31d7c6754ef61f2c02a","body_hash":"838d97c86b1f5f9587b4bd62ce5356e392f71645a124304077896286889b22fe","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.247"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"9bdf0d113775b734924255085705287647d2d4c6946b9311ec0fc3ea130cbe04","body_hash":"838d97c86b1f5f9587b4bd62ce5356e392f71645a124304077896286889b22fe","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.247"}} # gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"3d3c42e5aac5ba805825da76410c181273ba90b1","version":"v7.0.1"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-go","sha":"b7ad1dad31e06c5925ef5d2fc7ad053ef454303e","version":"v7.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"docker/build-push-action","sha":"53b7df96c91f9c12dcc8a07bcb9ccacbed38856a","version":"v7.3.0"},{"repo":"docker/setup-buildx-action","sha":"37fe631027851001ddb9b187196cc803df7f5f0e","version":"v4.3.0"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.28.13","digest":"sha256:78aeb1eee876d1ebae5eef26402bd9f31112850d42016a240002d1da57203e15","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.28.13@sha256:78aeb1eee876d1ebae5eef26402bd9f31112850d42016a240002d1da57203e15"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.13","digest":"sha256:61f12dcff38eb008b645d87a5541763452f409d9040eb243718764944167b1a2","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.13@sha256:61f12dcff38eb008b645d87a5541763452f409d9040eb243718764944167b1a2"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.28.13","digest":"sha256:87d9acf521959e7dae24bd8e5d972b836a0dd704f39f8b072c4fb01622a1c060","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.28.13@sha256:87d9acf521959e7dae24bd8e5d972b836a0dd704f39f8b072c4fb01622a1c060"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.28.13","digest":"sha256:58a8dc5be3fbafeef7da0532c38bd9047f86bf1b3c4cfd7d9b7d8d28e80a2b56","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.28.13@sha256:58a8dc5be3fbafeef7da0532c38bd9047f86bf1b3c4cfd7d9b7d8d28e80a2b56"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.15","digest":"sha256:60cd97533e93d8e7be36b979c0f08a70846189bda6190f28bbd6d427bc0d9b6e","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.15@sha256:60cd97533e93d8e7be36b979c0f08a70846189bda6190f28bbd6d427bc0d9b6e"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:bac2192f6374d6262116399b34fc5e143d576f82719e90a18261cae7480f4d4e","pinned_image":"ghcr.io/github/gh-aw-node@sha256:bac2192f6374d6262116399b34fc5e143d576f82719e90a18261cae7480f4d4e"},{"image":"ghcr.io/github/github-mcp-server:v1.11.0","digest":"sha256:fbec75de11c255213fa08d80fb166abe73d851fff631c51c0079872967720699","pinned_image":"ghcr.io/github/github-mcp-server:v1.11.0@sha256:fbec75de11c255213fa08d80fb166abe73d851fff631c51c0079872967720699"}],"mcp_servers":[{"name":"agenticworkflows","tools":["*"]},{"name":"safeoutputs","tools":["add_comment","create_issue","missing_data","missing_tool","noop"]}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -565,7 +565,9 @@ jobs: - name: Verify static analysis tools run: "set -e\necho \"Verifying static analysis tools are available...\"\n\n# Verify zizmor\necho \"Testing zizmor...\"\ndocker run --rm ghcr.io/zizmorcore/zizmor:latest --version || echo \"Warning: zizmor version check failed\"\n\n# Verify poutine\necho \"Testing poutine...\"\ndocker run --rm ghcr.io/boostsecurityio/poutine:latest --version || echo \"Warning: poutine version check failed\"\n\n# Verify runner-guard\necho \"Testing runner-guard...\"\ndocker run --rm ghcr.io/vigilant-llc/runner-guard:latest --version || echo \"Warning: runner-guard version check failed\"\n\n# Verify grype\necho \"Testing grype...\"\ndocker run --rm anchore/grype:latest version || echo \"Warning: grype version check failed\"\n\n# Verify syft\necho \"Testing syft...\"\ndocker run --rm anchore/syft:latest version || echo \"Warning: syft version check failed\"\n\n# Verify yamllint\necho \"Testing yamllint...\"\ndocker run --rm pipelinecomponents/yamllint:latest --version || echo \"Warning: yamllint version check failed\"\n\n# Verify shellcheck\necho \"Testing shellcheck...\"\ndocker run --rm koalaman/shellcheck:v0.11.0@sha256:61862eba1fcf09a484ebcc6feea46f1782532571a34ed51fedf90dd25f925a8d --version || echo \"Warning: shellcheck version check failed\"\n\necho \"Static analysis tools verification complete\"\n" - name: Run compile with security tools - run: "set -e\necho \"Running gh aw compile with security tools to download Docker images...\"\n\n# Run compile with all security scanner flags to download Docker images\n# Store the output in a file for inspection\n\"$GITHUB_WORKSPACE/gh-aw\" compile --zizmor --poutine --actionlint --runner-guard --syft --grype --yamllint --shellcheck 2>&1 | tee /tmp/gh-aw/agent/compile-output.txt\n\necho \"Compile with security tools completed\"\necho \"Output saved to /tmp/gh-aw/agent/compile-output.txt\"" + run: "set -e\necho \"Running gh aw compile with security tools to download Docker images...\"\n\n# Run compile with all security scanner flags to download Docker images\n# Store the output in a file for inspection\n\"$GITHUB_WORKSPACE/gh-aw\" compile --zizmor --poutine --actionlint --runner-guard --syft --grype --yamllint --shellcheck 2>&1 | tee /tmp/gh-aw/agent/compile-output.txt\n\necho \"Compile with security tools completed\"\necho \"Output saved to /tmp/gh-aw/agent/compile-output.txt\"\n" + - name: Assert static analysis output completeness + run: "set -e\necho \"Verifying all static analysis tools executed and produced output...\"\nCOMPILE_LOG=\"/tmp/gh-aw/agent/compile-output.txt\"\n\nMISSING_TOOLS=0\nfor tool in zizmor poutine actionlint runner-guard syft grype yamllint shellcheck; do\n if ! grep -qi \"$tool\" \"$COMPILE_LOG\"; then\n echo \"Error: Static analysis tool '$tool' produced zero output in $COMPILE_LOG\"\n MISSING_TOOLS=$((MISSING_TOOLS + 1))\n fi\ndone\n\nif [ $MISSING_TOOLS -gt 0 ]; then\n echo \"Error: $MISSING_TOOLS static analysis tool(s) failed to produce execution output in pipeline\"\n exit 1\nfi\n\necho \"Static analysis tool output completeness check passed.\"" - name: Configure Git credentials env: diff --git a/.github/workflows/static-analysis-report.md b/.github/workflows/static-analysis-report.md index d28b90603e7..5b7dabc4acd 100644 --- a/.github/workflows/static-analysis-report.md +++ b/.github/workflows/static-analysis-report.md @@ -119,6 +119,26 @@ steps: echo "Compile with security tools completed" echo "Output saved to /tmp/gh-aw/agent/compile-output.txt" + - name: Assert static analysis output completeness + run: | + set -e + echo "Verifying all static analysis tools executed and produced output..." + COMPILE_LOG="/tmp/gh-aw/agent/compile-output.txt" + + MISSING_TOOLS=0 + for tool in zizmor poutine actionlint runner-guard syft grype yamllint shellcheck; do + if ! grep -qi "$tool" "$COMPILE_LOG"; then + echo "Error: Static analysis tool '$tool' produced zero output in $COMPILE_LOG" + MISSING_TOOLS=$((MISSING_TOOLS + 1)) + fi + done + + if [ $MISSING_TOOLS -gt 0 ]; then + echo "Error: $MISSING_TOOLS static analysis tool(s) failed to produce execution output in pipeline" + exit 1 + fi + + echo "Static analysis tool output completeness check passed." sandbox: agent: diff --git a/pkg/cli/actionlint.go b/pkg/cli/actionlint.go index c312cbd5c52..525ae0310fb 100644 --- a/pkg/cli/actionlint.go +++ b/pkg/cli/actionlint.go @@ -236,6 +236,7 @@ func runActionlintOnFilesWithOptions(ctx context.Context, lockFiles []string, ve return nil } actionlintLog.Printf("Running actionlint on %d file(s): %v (verbose=%t, strict=%t)", len(lockFiles), lockFiles, verbose, strict) + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Running actionlint on %d file(s)", len(lockFiles)))) maybePrintActionlintVersion(ctx) gitRoot, relPaths, err := resolveActionlintPaths(lockFiles) diff --git a/pkg/cli/compile_external_tools_test.go b/pkg/cli/compile_external_tools_test.go index 3cc1625fee1..a8b1cbf6c37 100644 --- a/pkg/cli/compile_external_tools_test.go +++ b/pkg/cli/compile_external_tools_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "errors" "testing" ) @@ -36,3 +37,40 @@ func TestHandleBatchToolErrorPropagatesInStrictMode(t *testing.T) { t.Fatal("expected strict mode to propagate errors, got nil") } } + +func TestRunBatchExternalToolsExecutesSequentialToolsWithoutEarlyAborting(t *testing.T) { + t.Parallel() + + // Given a context and empty lock file options + ctx := context.Background() + config := CompileConfig{ + Actionlint: true, + Zizmor: true, + Poutine: true, + RunnerGuard: true, + Syft: true, + Grype: true, + Yamllint: true, + } + + opts := batchToolsOptions{ + workflowDir: t.TempDir(), + lockFilesForActionlint: []string{}, + lockFilesForZizmor: []string{}, + lockFilesForDirTools: []string{}, + lockFilesForSyft: []string{}, + lockFilesForGrype: []string{}, + lockFilesForYamllint: []string{}, + } + + stats := &CompilationStats{} + var validationResults []ValidationResult + + strictGrantErr, batchToolErr := runBatchExternalTools(ctx, config, opts, stats, &validationResults) + if strictGrantErr != nil { + t.Fatalf("expected no strictGrantErr, got %v", strictGrantErr) + } + if batchToolErr != nil { + t.Fatalf("expected no batchToolErr for empty lock files, got %v", batchToolErr) + } +} diff --git a/pkg/cli/compile_pipeline.go b/pkg/cli/compile_pipeline.go index cb6718a2a76..f661a5478c6 100644 --- a/pkg/cli/compile_pipeline.go +++ b/pkg/cli/compile_pipeline.go @@ -189,128 +189,20 @@ func compileSpecificFiles( //nolint:largefunc // Orchestrates the full targeted *validationResults = append(*validationResults, fileResult.validationResult) } - // Run batch actionlint on all collected lock files - if config.Actionlint && !config.NoEmit && len(lockFilesForActionlint) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := RunActionlintOnFiles(ctx, lockFilesForActionlint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err - } - } - } - - // Run batch zizmor on all collected lock files - if config.Zizmor && !config.NoEmit && len(lockFilesForZizmor) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := RunZizmorOnFiles(lockFilesForZizmor, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - // Always fail on high/critical severity findings (zizmor returns errors for those - // regardless of strict mode). In strict mode, all findings are errors. - return workflowDataList, err - } - } - - // Run batch poutine once on the workflow directory - // Get the directory from the first lock file (all should be in same directory) - if config.Poutine && !config.NoEmit && len(lockFilesForDirTools) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - workflowDir := filepath.Dir(lockFilesForDirTools[0]) - if err := runBatchDirectoryTool("poutine", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, RunPoutineOnDirectory); err != nil { - if config.Strict { - return workflowDataList, err - } - } - } - - // Run batch runner-guard once on the workflow directory - // Get the directory from the first lock file (all should be in same directory) - if config.RunnerGuard && !config.NoEmit && len(lockFilesForDirTools) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - workflowDir := filepath.Dir(lockFilesForDirTools[0]) - if err := runBatchDirectoryTool("runner-guard", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, RunRunnerGuardOnDirectory); err != nil { - if config.Strict { - return workflowDataList, err - } - } - } - - // Run syft SBOM scanner on container images referenced in the compiled lock files. - if config.Syft && !config.NoEmit && len(lockFilesForSyft) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := RunSyftOnLockFiles(lockFilesForSyft, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err - } - } - } - - // Run grype vulnerability scanner on container images referenced in the compiled lock files. - if config.Grype && !config.NoEmit && len(lockFilesForGrype) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := RunGrypeOnLockFiles(lockFilesForGrype, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err - } - } - } - - // Run grant license scanner on container images referenced in the compiled lock files. - if config.Grant && !config.NoEmit && len(lockFilesForGrant) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := RunGrantOnLockFiles(lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - errorCount++ - stats.Errors++ - // Grant is a post-compilation tool, not a workflow; record it only in - // validationResults (for JSON output) without adding to FailureDetails. - *validationResults = append(*validationResults, ValidationResult{ - Workflow: "grant", - Valid: false, - Errors: []ValidationIssue{{ - Type: "grant_error", - Message: err.Error(), - }}, - }) - strictGrantErr = err - } - } - } - - // Run yamllint on all collected lock files. - if config.Yamllint && !config.NoEmit && len(lockFilesForYamllint) > 0 { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := runBatchYamllintOnFiles(lockFilesForYamllint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err - } - } - } - - // Run shellcheck on run step scripts in all collected lock files. - if config.shellcheckEnabled() && !config.NoEmit && (len(lockFilesForShellcheck) > 0 || len(shellcheckResources) > 0) { - if err := ctx.Err(); err != nil { - return workflowDataList, err - } - if err := RunShellcheckOnLockFilesAndResources(ctx, lockFilesForShellcheck, shellcheckResources, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err - } - } + strictGrantErr, batchToolErr := runBatchExternalTools(ctx, config, batchToolsOptions{ + lockFilesForActionlint: lockFilesForActionlint, + lockFilesForZizmor: lockFilesForZizmor, + lockFilesForDirTools: lockFilesForDirTools, + lockFilesForSyft: lockFilesForSyft, + lockFilesForGrype: lockFilesForGrype, + lockFilesForGrant: lockFilesForGrant, + lockFilesForYamllint: lockFilesForYamllint, + lockFilesForShellcheck: lockFilesForShellcheck, + shellcheckResources: shellcheckResources, + }, stats, validationResults) + + if strictGrantErr != nil || batchToolErr != nil { + errorCount++ } // Get warning count from compiler @@ -342,6 +234,9 @@ func compileSpecificFiles( //nolint:largefunc // Orchestrates the full targeted if strictGrantErr != nil { return workflowDataList, strictGrantErr } + if batchToolErr != nil { + return workflowDataList, batchToolErr + } return workflowDataList, errors.New("compilation failed") } @@ -508,87 +403,200 @@ func compileAllFilesInDirectory( //nolint:largefunc // Orchestrates the full dir *validationResults = append(*validationResults, fileResult.validationResult) } - // Run batch actionlint - if config.Actionlint && !config.NoEmit && len(lockFilesForActionlint) > 0 { + strictGrantErr, batchToolErr := runBatchExternalTools(ctx, config, batchToolsOptions{ + workflowDir: workflowsDir, + lockFilesForActionlint: lockFilesForActionlint, + lockFilesForZizmor: lockFilesForZizmor, + lockFilesForDirTools: lockFilesForDirTools, + lockFilesForSyft: lockFilesForSyft, + lockFilesForGrype: lockFilesForGrype, + lockFilesForGrant: lockFilesForGrant, + lockFilesForYamllint: lockFilesForYamllint, + lockFilesForShellcheck: lockFilesForShellcheck, + shellcheckResources: shellcheckResources, + }, stats, validationResults) + + if strictGrantErr != nil || batchToolErr != nil { + errorCount++ + } + + // Emit recommendation when many slash commands are present without centralized strategy. + displayCentralizedSlashCommandRecommendation(compiler, workflowDataList, config.JSONOutput) + + duplicateNameWarnings, err := appendDuplicateWorkflowNameWarnings(workflowDataList, workflowValidationResultIndexes, validationResults) + if err != nil { + return workflowDataList, err + } + if !config.JSONOutput { + for _, warning := range duplicateNameWarnings { + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warning.Message)) + } + } + + // Get warning count from compiler + stats.Warnings = compiler.GetWarningCount() + len(duplicateNameWarnings) + + displayBatchCompilationNotices(compiler, config) + + // Display schedule warnings + displayScheduleWarnings(compiler, config.JSONOutput) + + // Display safe update warnings (emitted as prompts for the calling agent) + displaySafeUpdateWarnings(compiler, config.JSONOutput) + + if config.Verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(fmt.Sprintf("Successfully compiled %d out of %d workflow files", successCount, len(mdFiles)))) + } + + // Handle purge logic if requested + if config.Purge && purgeData != nil { + runPurgeOperations(workflowsDir, purgeData, config.Verbose) + } + + // Post-processing + if err := runPostProcessingForDirectory(ctx, compiler, workflowDataList, config, workflowsDir, gitRoot, successCount, errorCount); err != nil { + return workflowDataList, err + } + + // Output results. + // Populate MarkdownFiles so that outputResults can collect per-workflow stats + // (e.g. schedule heatmap) even when the caller did not specify explicit files. + if config.Stats && len(config.MarkdownFiles) == 0 { + config.MarkdownFiles = mdFiles + } + if err := outputResults(stats, validationResults, config); err != nil { + return workflowDataList, err + } + + // Return error if any compilations failed + if errorCount > 0 { + if strictGrantErr != nil { + return workflowDataList, strictGrantErr + } + if batchToolErr != nil { + return workflowDataList, batchToolErr + } + return workflowDataList, errors.New("compilation failed") + } + + return workflowDataList, nil +} + +type batchToolsOptions struct { + workflowDir string + lockFilesForActionlint []string + lockFilesForZizmor []string + lockFilesForDirTools []string + lockFilesForSyft []string + lockFilesForGrype []string + lockFilesForGrant []string + lockFilesForYamllint []string + lockFilesForShellcheck []string + shellcheckResources []workflow.ShellScriptResource +} + +// runBatchExternalTools executes all enabled batch analysis tools sequentially without short-circuiting +// when individual tools report findings or errors. +func runBatchLinters(ctx context.Context, config CompileConfig, opts batchToolsOptions) error { + var firstErr error + + if config.Actionlint && !config.NoEmit && len(opts.lockFilesForActionlint) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return err } - if err := RunActionlintOnFiles(ctx, lockFilesForActionlint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err + if err := RunActionlintOnFiles(ctx, opts.lockFilesForActionlint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if config.Strict && firstErr == nil { + firstErr = err } } } - // Run batch zizmor - if config.Zizmor && !config.NoEmit && len(lockFilesForZizmor) > 0 { + if config.Zizmor && !config.NoEmit && len(opts.lockFilesForZizmor) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return err } - if err := RunZizmorOnFiles(lockFilesForZizmor, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - return workflowDataList, err + if err := RunZizmorOnFiles(opts.lockFilesForZizmor, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if firstErr == nil { + firstErr = err + } } } - // Run batch poutine once on the workflow directory - if config.Poutine && !config.NoEmit && len(lockFilesForDirTools) > 0 { + return firstErr +} + +func runBatchDirScanners(ctx context.Context, config CompileConfig, opts batchToolsOptions) error { + var firstErr error + + if config.Poutine && !config.NoEmit && len(opts.lockFilesForDirTools) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return err } - if err := runBatchDirectoryTool("poutine", workflowsDir, config.Verbose && !config.JSONOutput, config.Strict, RunPoutineOnDirectory); err != nil { - if config.Strict { - return workflowDataList, err + workflowDir := opts.workflowDir + if workflowDir == "" && len(opts.lockFilesForDirTools) > 0 { + workflowDir = filepath.Dir(opts.lockFilesForDirTools[0]) + } + if err := runBatchDirectoryTool("poutine", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, RunPoutineOnDirectory); err != nil { + if config.Strict && firstErr == nil { + firstErr = err } } } - // Run batch runner-guard once on the workflow directory - if config.RunnerGuard && !config.NoEmit && len(lockFilesForDirTools) > 0 { + if config.RunnerGuard && !config.NoEmit && len(opts.lockFilesForDirTools) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return err } - if err := runBatchDirectoryTool("runner-guard", workflowsDir, config.Verbose && !config.JSONOutput, config.Strict, RunRunnerGuardOnDirectory); err != nil { - if config.Strict { - return workflowDataList, err + workflowDir := opts.workflowDir + if workflowDir == "" && len(opts.lockFilesForDirTools) > 0 { + workflowDir = filepath.Dir(opts.lockFilesForDirTools[0]) + } + if err := runBatchDirectoryTool("runner-guard", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, RunRunnerGuardOnDirectory); err != nil { + if config.Strict && firstErr == nil { + firstErr = err } } } - // Run syft SBOM scanner on container images referenced in the compiled lock files. - if config.Syft && !config.NoEmit && len(lockFilesForSyft) > 0 { + return firstErr +} + +func runBatchContainerScanners( + ctx context.Context, + config CompileConfig, + opts batchToolsOptions, + stats *CompilationStats, + validationResults *[]ValidationResult, +) (strictGrantErr error, containerErr error) { + if config.Syft && !config.NoEmit && len(opts.lockFilesForSyft) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return nil, err } - if err := RunSyftOnLockFiles(lockFilesForSyft, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err + if err := RunSyftOnLockFiles(opts.lockFilesForSyft, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if config.Strict && containerErr == nil { + containerErr = err } } } - // Run grype vulnerability scanner on container images referenced in the compiled lock files. - if config.Grype && !config.NoEmit && len(lockFilesForGrype) > 0 { + if config.Grype && !config.NoEmit && len(opts.lockFilesForGrype) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return nil, err } - if err := RunGrypeOnLockFiles(lockFilesForGrype, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err + if err := RunGrypeOnLockFiles(opts.lockFilesForGrype, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if config.Strict && containerErr == nil { + containerErr = err } } } - // Run grant license scanner on container images referenced in the compiled lock files. - if config.Grant && !config.NoEmit && len(lockFilesForGrant) > 0 { + if config.Grant && !config.NoEmit && len(opts.lockFilesForGrant) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return nil, err } - if err := RunGrantOnLockFiles(lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := RunGrantOnLockFiles(opts.lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if config.Strict { - errorCount++ stats.Errors++ - // Grant is a post-compilation tool, not a workflow; record it only in - // validationResults (for JSON output) without adding to FailureDetails. *validationResults = append(*validationResults, ValidationResult{ Workflow: "grant", Valid: false, @@ -597,92 +605,72 @@ func compileAllFilesInDirectory( //nolint:largefunc // Orchestrates the full dir Message: err.Error(), }}, }) - strictGrantErr = err + if strictGrantErr == nil { + strictGrantErr = err + } } } } - // Run batch yamllint - if config.Yamllint && !config.NoEmit && len(lockFilesForYamllint) > 0 { + return strictGrantErr, containerErr +} + +func runBatchScriptLinters(ctx context.Context, config CompileConfig, opts batchToolsOptions) error { + var firstErr error + + if config.Yamllint && !config.NoEmit && len(opts.lockFilesForYamllint) > 0 { if err := ctx.Err(); err != nil { - return workflowDataList, err + return err } - if err := runBatchYamllintOnFiles(lockFilesForYamllint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err + if err := runBatchYamllintOnFiles(opts.lockFilesForYamllint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if config.Strict && firstErr == nil { + firstErr = err } } } - // Run shellcheck on run step scripts in all collected lock files. - if config.shellcheckEnabled() && !config.NoEmit && (len(lockFilesForShellcheck) > 0 || len(shellcheckResources) > 0) { + if config.shellcheckEnabled() && !config.NoEmit && (len(opts.lockFilesForShellcheck) > 0 || len(opts.shellcheckResources) > 0) { if err := ctx.Err(); err != nil { - return workflowDataList, err + return err } - if err := RunShellcheckOnLockFilesAndResources(ctx, lockFilesForShellcheck, shellcheckResources, config.Verbose && !config.JSONOutput, config.Strict); err != nil { - if config.Strict { - return workflowDataList, err + if err := RunShellcheckOnLockFilesAndResources(ctx, opts.lockFilesForShellcheck, opts.shellcheckResources, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if config.Strict && firstErr == nil { + firstErr = err } } } - // Emit recommendation when many slash commands are present without centralized strategy. - displayCentralizedSlashCommandRecommendation(compiler, workflowDataList, config.JSONOutput) - - duplicateNameWarnings, err := appendDuplicateWorkflowNameWarnings(workflowDataList, workflowValidationResultIndexes, validationResults) - if err != nil { - return workflowDataList, err - } - if !config.JSONOutput { - for _, warning := range duplicateNameWarnings { - fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warning.Message)) - } - } - - // Get warning count from compiler - stats.Warnings = compiler.GetWarningCount() + len(duplicateNameWarnings) - - displayBatchCompilationNotices(compiler, config) - - // Display schedule warnings - displayScheduleWarnings(compiler, config.JSONOutput) - - // Display safe update warnings (emitted as prompts for the calling agent) - displaySafeUpdateWarnings(compiler, config.JSONOutput) - - if config.Verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(fmt.Sprintf("Successfully compiled %d out of %d workflow files", successCount, len(mdFiles)))) - } + return firstErr +} - // Handle purge logic if requested - if config.Purge && purgeData != nil { - runPurgeOperations(workflowsDir, purgeData, config.Verbose) +func runBatchExternalTools( + ctx context.Context, + config CompileConfig, + opts batchToolsOptions, + stats *CompilationStats, + validationResults *[]ValidationResult, +) (strictGrantErr error, batchToolErr error) { + if err := runBatchLinters(ctx, config, opts); err != nil && batchToolErr == nil { + batchToolErr = err } - // Post-processing - if err := runPostProcessingForDirectory(ctx, compiler, workflowDataList, config, workflowsDir, gitRoot, successCount, errorCount); err != nil { - return workflowDataList, err + if err := runBatchDirScanners(ctx, config, opts); err != nil && batchToolErr == nil { + batchToolErr = err } - // Output results. - // Populate MarkdownFiles so that outputResults can collect per-workflow stats - // (e.g. schedule heatmap) even when the caller did not specify explicit files. - if config.Stats && len(config.MarkdownFiles) == 0 { - config.MarkdownFiles = mdFiles + sGrantErr, containerErr := runBatchContainerScanners(ctx, config, opts, stats, validationResults) + if sGrantErr != nil && strictGrantErr == nil { + strictGrantErr = sGrantErr } - if err := outputResults(stats, validationResults, config); err != nil { - return workflowDataList, err + if containerErr != nil && batchToolErr == nil { + batchToolErr = containerErr } - // Return error if any compilations failed - if errorCount > 0 { - if strictGrantErr != nil { - return workflowDataList, strictGrantErr - } - return workflowDataList, errors.New("compilation failed") + if err := runBatchScriptLinters(ctx, config, opts); err != nil && batchToolErr == nil { + batchToolErr = err } - return workflowDataList, nil + return strictGrantErr, batchToolErr } func appendDuplicateWorkflowNameWarnings(workflowDataList []*workflow.WorkflowData, validationResultIndexes []int, validationResults *[]ValidationResult) ([]ValidationIssue, error) { diff --git a/pkg/cli/grype.go b/pkg/cli/grype.go index dd84d3108a3..9c83f1c9be0 100644 --- a/pkg/cli/grype.go +++ b/pkg/cli/grype.go @@ -177,9 +177,7 @@ func runGrypeOnLockFiles(lockFiles []string, verbose bool, strict bool) error { images := collectContainerImagesFromLockFiles(lockFiles) if len(images) == 0 { grypeLog.Print("No container images found in lock files") - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("No container images found in lock files to scan with grype")) - } + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running grype vulnerability scanner (0 container images found in lock files)")) return nil } diff --git a/pkg/cli/shellcheck.go b/pkg/cli/shellcheck.go index 660248b1c68..8bbdcb79e75 100644 --- a/pkg/cli/shellcheck.go +++ b/pkg/cli/shellcheck.go @@ -143,6 +143,46 @@ func resolveDefaultShell(defaults map[string]any) string { return shell } +// extractRunStepsFromLockFile parses a compiled lock file and returns all +// run: steps whose effective shell is lintable by shellcheck. +// +// The effective shell for a step is resolved in priority order: +func extractRunStepsFromJob(job map[string]any, jobDefaultShell, lockFile string) []runStepInfo { + var steps []runStepInfo + rawSteps, ok := job["steps"].([]any) + if !ok { + return steps + } + for _, stepData := range rawSteps { + step, ok := stepData.(map[string]any) + if !ok { + continue + } + runScript, ok := step["run"].(string) + if !ok || runScript == "" { + continue + } + + shell, _ := step["shell"].(string) + effectiveShell := shell + if effectiveShell == "" { + effectiveShell = jobDefaultShell + } + + if !isShellcheckableShell(effectiveShell) { + continue + } + name, _ := step["name"].(string) + steps = append(steps, runStepInfo{ + Name: name, + Script: runScript, + Shell: effectiveShell, + LockFile: lockFile, + }) + } + return steps +} + // extractRunStepsFromLockFile parses a compiled lock file and returns all // run: steps whose effective shell is lintable by shellcheck. // @@ -191,44 +231,30 @@ func extractRunStepsFromLockFile(lockFile string) ([]runStepInfo, error) { } } - rawSteps, ok := job["steps"].([]any) - if !ok { - continue - } - for _, stepData := range rawSteps { - step, ok := stepData.(map[string]any) - if !ok { - continue - } - runScript, ok := step["run"].(string) - if !ok || runScript == "" { - continue - } - - // Resolve effective shell: step > job default > workflow default. - shell, _ := step["shell"].(string) - effectiveShell := shell - if effectiveShell == "" { - effectiveShell = jobDefaultShell - } - - if !isShellcheckableShell(effectiveShell) { - continue - } - name, _ := step["name"].(string) - steps = append(steps, runStepInfo{ - Name: name, - Script: runScript, - Shell: effectiveShell, - LockFile: lockFile, - }) - } + steps = append(steps, extractRunStepsFromJob(job, jobDefaultShell, lockFile)...) } shellcheckLog.Printf("Found %d shellcheckable run steps in %s", len(steps), lockFile) return steps, nil } +func writeTempScriptFile(script string) (string, error) { + // Sanitize GitHub Actions ${{ ... }} expressions before writing the script. + sanitizedScript := sanitizeGHAExpressions(script) + + tmpFile, err := os.CreateTemp("", "gh-aw-shellcheck-*.sh") + if err != nil { + return "", fmt.Errorf("failed to create temp file for shellcheck: %w", err) + } + if _, err := tmpFile.WriteString(sanitizedScript); err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + return "", fmt.Errorf("failed to write shellcheck temp file: %w", err) + } + tmpFile.Close() + return tmpFile.Name(), nil +} + // runShellcheckOnScript writes script to a temporary file and invokes shellcheck. // It returns any findings as a byte slice (ready to write to stderr) and a // non-nil error when shellcheck reports one or more issues. Callers are @@ -237,25 +263,11 @@ func extractRunStepsFromLockFile(lockFile string) ([]runStepInfo, error) { func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) ([]byte, error) { shellcheckLog.Printf("Running shellcheck on step %q (shell=%s)", info.Name, info.Shell) - var out bytes.Buffer - - // Sanitize GitHub Actions ${{ ... }} expressions before writing the script. - // Without this, shellcheck emits parse errors (SC1073, SC1083) because - // ${{ is not valid POSIX/bash substitution syntax. - sanitizedScript := sanitizeGHAExpressions(info.Script) - - // Write script to a temp file so shellcheck can lint it. - tmpFile, err := os.CreateTemp("", "gh-aw-shellcheck-*.sh") + tmpPath, err := writeTempScriptFile(info.Script) if err != nil { - return nil, fmt.Errorf("failed to create temp file for shellcheck: %w", err) - } - defer os.Remove(tmpFile.Name()) - - if _, err := tmpFile.WriteString(sanitizedScript); err != nil { - tmpFile.Close() - return nil, fmt.Errorf("failed to write shellcheck temp file: %w", err) + return nil, err } - tmpFile.Close() + defer os.Remove(tmpPath) args := []string{ "--shell=" + shellcheckShell(info.Shell), @@ -264,7 +276,7 @@ func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) for _, code := range ignoreCodes { args = append(args, "--exclude="+code) } - args = append(args, tmpFile.Name()) + args = append(args, tmpPath) if verbose { shellcheckLog.Printf("Invoking: shellcheck %s", strings.Join(args, " ")) @@ -278,31 +290,30 @@ func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) cmd.Stdout = &stdout cmd.Stderr = &stderr - err = cmd.Run() + cmdErr := cmd.Run() // Replace the temp file path with "script" so that reported line numbers // are clearly relative to the run: script snippet rather than the lock // file. The enclosing header message ("shellcheck findings in ") // already identifies the originating file and step. - findings := strings.ReplaceAll(stdout.String(), tmpFile.Name(), "script") + findings := strings.ReplaceAll(stdout.String(), tmpPath, "script") if stderr.Len() > 0 { - findings += strings.ReplaceAll(stderr.String(), tmpFile.Name(), "script") + findings += strings.ReplaceAll(stderr.String(), tmpPath, "script") } + var out bytes.Buffer if findings != "" { fmt.Fprintf(&out, "%s\n", console.FormatWarningMessage("shellcheck findings in "+stepLabel(info)+":")) fmt.Fprint(&out, findings) } - if err != nil { + if cmdErr != nil { var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - if exitErr.ExitCode() == 1 { - // Exit code 1 means shellcheck found issues; already printed above. - return out.Bytes(), fmt.Errorf("shellcheck found issues in %s", stepLabel(info)) - } + if errors.As(cmdErr, &exitErr) && exitErr.ExitCode() == 1 { + // Exit code 1 means shellcheck found issues; already printed above. + return out.Bytes(), fmt.Errorf("shellcheck found issues in %s", stepLabel(info)) } - return out.Bytes(), fmt.Errorf("shellcheck failed: %w", err) + return out.Bytes(), fmt.Errorf("shellcheck failed: %w", cmdErr) } return out.Bytes(), nil @@ -430,6 +441,7 @@ func runShellcheckOnLockFilesAndResources(ctx context.Context, lockFiles []strin allSteps := collectShellcheckSteps(lockFiles, resources) if len(allSteps) == 0 { + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running shellcheck on run steps (0 run steps found in lock files)")) return nil } return runShellcheckOnSteps(ctx, allSteps, lockFiles, verbose, strict, useDocker) diff --git a/pkg/cli/syft.go b/pkg/cli/syft.go index 9dae5fd9a0d..4deb7b3480e 100644 --- a/pkg/cli/syft.go +++ b/pkg/cli/syft.go @@ -44,9 +44,7 @@ func runSyftOnLockFiles(lockFiles []string, verbose bool, strict bool) error { / images := collectContainerImagesFromLockFiles(lockFiles) if len(images) == 0 { syftLog.Print("No container images found in lock files") - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("No container images found in lock files to scan with syft")) - } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running syft SBOM scanner (0 container images found in lock files)")) return nil } From ffd49a91c1e90b0a549a7bf68c7daac0c9bfdd18 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 05:46:00 +0000 Subject: [PATCH 3/3] Address review feedback: scanner-specific markers, injectable seams, zero-input logging, doc fix Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../workflows/static-analysis-report.lock.yml | 4 +- .github/workflows/static-analysis-report.md | 19 +++- pkg/cli/compile_external_tools.go | 7 +- pkg/cli/compile_external_tools_test.go | 105 ++++++++++++++++-- pkg/cli/compile_pipeline.go | 51 ++++++--- pkg/cli/shellcheck.go | 8 +- 6 files changed, 157 insertions(+), 37 deletions(-) diff --git a/.github/workflows/static-analysis-report.lock.yml b/.github/workflows/static-analysis-report.lock.yml index 032b9ec53b9..42cf85ee774 100644 --- a/.github/workflows/static-analysis-report.lock.yml +++ b/.github/workflows/static-analysis-report.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"0447034e65ed87b9db93a1d314aa401e075c7836c0ab9ec26cfea8e4331a793b","body_hash":"838d97c86b1f5f9587b4bd62ce5356e392f71645a124304077896286889b22fe","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.247"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"448a6bb6a1a67ff2b8f4c9df1588e1ce8990a00a92e08ec60388c6387f1d7627","body_hash":"838d97c86b1f5f9587b4bd62ce5356e392f71645a124304077896286889b22fe","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.247"}} # gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"3d3c42e5aac5ba805825da76410c181273ba90b1","version":"v7.0.1"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-go","sha":"b7ad1dad31e06c5925ef5d2fc7ad053ef454303e","version":"v7.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"docker/build-push-action","sha":"53b7df96c91f9c12dcc8a07bcb9ccacbed38856a","version":"v7.3.0"},{"repo":"docker/setup-buildx-action","sha":"37fe631027851001ddb9b187196cc803df7f5f0e","version":"v4.3.0"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.28.13","digest":"sha256:78aeb1eee876d1ebae5eef26402bd9f31112850d42016a240002d1da57203e15","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.28.13@sha256:78aeb1eee876d1ebae5eef26402bd9f31112850d42016a240002d1da57203e15"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.13","digest":"sha256:61f12dcff38eb008b645d87a5541763452f409d9040eb243718764944167b1a2","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.28.13@sha256:61f12dcff38eb008b645d87a5541763452f409d9040eb243718764944167b1a2"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.28.13","digest":"sha256:58a8dc5be3fbafeef7da0532c38bd9047f86bf1b3c4cfd7d9b7d8d28e80a2b56","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.28.13@sha256:58a8dc5be3fbafeef7da0532c38bd9047f86bf1b3c4cfd7d9b7d8d28e80a2b56"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.15","digest":"sha256:60cd97533e93d8e7be36b979c0f08a70846189bda6190f28bbd6d427bc0d9b6e","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.15@sha256:60cd97533e93d8e7be36b979c0f08a70846189bda6190f28bbd6d427bc0d9b6e"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:bac2192f6374d6262116399b34fc5e143d576f82719e90a18261cae7480f4d4e","pinned_image":"ghcr.io/github/gh-aw-node@sha256:bac2192f6374d6262116399b34fc5e143d576f82719e90a18261cae7480f4d4e"},{"image":"ghcr.io/github/github-mcp-server:v1.11.0","digest":"sha256:fbec75de11c255213fa08d80fb166abe73d851fff631c51c0079872967720699","pinned_image":"ghcr.io/github/github-mcp-server:v1.11.0@sha256:fbec75de11c255213fa08d80fb166abe73d851fff631c51c0079872967720699"}],"mcp_servers":[{"name":"agenticworkflows","tools":["*"]},{"name":"github","tools":["actions_get","actions_list","get_commit","get_file_contents","get_job_logs","get_latest_release","get_me","get_pull_request","get_pull_request_comments","get_pull_request_diff","get_pull_request_files","get_pull_request_review_comments","get_pull_request_reviews","get_pull_request_status","get_release_by_tag","get_tag","issue_read","list_branches","list_commits","list_issue_types","list_issues","list_pull_requests","list_releases","list_starred_repositories","list_tags","pull_request_read","search_code","search_issues","search_pull_requests","search_repositories"]},{"name":"safeoutputs","tools":["add_comment","create_issue","missing_data","missing_tool","noop"]}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -566,7 +566,7 @@ jobs: - name: Run compile with security tools run: "set -e\necho \"Running gh aw compile with security tools to download Docker images...\"\n\n# Run compile with all security scanner flags to download Docker images\n# Store the output in a file for inspection\n\"$GITHUB_WORKSPACE/gh-aw\" compile --zizmor --poutine --actionlint --runner-guard --syft --grype --yamllint --shellcheck 2>&1 | tee /tmp/gh-aw/agent/compile-output.txt\n\necho \"Compile with security tools completed\"\necho \"Output saved to /tmp/gh-aw/agent/compile-output.txt\"\n" - name: Assert static analysis output completeness - run: "set -e\necho \"Verifying all static analysis tools executed and produced output...\"\nCOMPILE_LOG=\"/tmp/gh-aw/agent/compile-output.txt\"\n\nMISSING_TOOLS=0\nfor tool in zizmor poutine actionlint runner-guard syft grype yamllint shellcheck; do\n if ! grep -qi \"$tool\" \"$COMPILE_LOG\"; then\n echo \"Error: Static analysis tool '$tool' produced zero output in $COMPILE_LOG\"\n MISSING_TOOLS=$((MISSING_TOOLS + 1))\n fi\ndone\n\nif [ $MISSING_TOOLS -gt 0 ]; then\n echo \"Error: $MISSING_TOOLS static analysis tool(s) failed to produce execution output in pipeline\"\n exit 1\nfi\n\necho \"Static analysis tool output completeness check passed.\"" + run: "set -e\necho \"Verifying all static analysis tools executed and produced output...\"\nCOMPILE_LOG=\"/tmp/gh-aw/agent/compile-output.txt\"\n\n# Each tool has a unique, scanner-specific invocation marker so this check cannot\n# be satisfied by another tool's log output (e.g. actionlint's summary mentions\n# \"shellcheck/pyflakes\" but never emits the dedicated shellcheck marker below).\ndeclare -A TOOL_MARKERS=(\n [zizmor]=\"Running zizmor\"\n [poutine]=\"Running poutine security scanner\"\n [actionlint]=\"Running actionlint (\"\n [runner-guard]=\"Running runner-guard taint analysis\"\n [syft]=\"Running syft\"\n [grype]=\"Running grype\"\n [yamllint]=\"Running yamllint\"\n [shellcheck]=\"Running shellcheck on\"\n)\n\nMISSING_TOOLS=0\nfor tool in zizmor poutine actionlint runner-guard syft grype yamllint shellcheck; do\n marker=\"${TOOL_MARKERS[$tool]}\"\n if ! grep -qF \"$marker\" \"$COMPILE_LOG\"; then\n echo \"Error: Static analysis tool '$tool' produced zero output (missing marker: \\\"$marker\\\") in $COMPILE_LOG\"\n MISSING_TOOLS=$((MISSING_TOOLS + 1))\n fi\ndone\n\nif [ $MISSING_TOOLS -gt 0 ]; then\n echo \"Error: $MISSING_TOOLS static analysis tool(s) failed to produce execution output in pipeline\"\n exit 1\nfi\n\necho \"Static analysis tool output completeness check passed.\"" - name: Configure Git credentials env: diff --git a/.github/workflows/static-analysis-report.md b/.github/workflows/static-analysis-report.md index b676ac1abc0..bf93c838ad0 100644 --- a/.github/workflows/static-analysis-report.md +++ b/.github/workflows/static-analysis-report.md @@ -127,10 +127,25 @@ steps: echo "Verifying all static analysis tools executed and produced output..." COMPILE_LOG="/tmp/gh-aw/agent/compile-output.txt" + # Each tool has a unique, scanner-specific invocation marker so this check cannot + # be satisfied by another tool's log output (e.g. actionlint's summary mentions + # "shellcheck/pyflakes" but never emits the dedicated shellcheck marker below). + declare -A TOOL_MARKERS=( + [zizmor]="Running zizmor" + [poutine]="Running poutine security scanner" + [actionlint]="Running actionlint (" + [runner-guard]="Running runner-guard taint analysis" + [syft]="Running syft" + [grype]="Running grype" + [yamllint]="Running yamllint" + [shellcheck]="Running shellcheck on" + ) + MISSING_TOOLS=0 for tool in zizmor poutine actionlint runner-guard syft grype yamllint shellcheck; do - if ! grep -qi "$tool" "$COMPILE_LOG"; then - echo "Error: Static analysis tool '$tool' produced zero output in $COMPILE_LOG" + marker="${TOOL_MARKERS[$tool]}" + if ! grep -qF "$marker" "$COMPILE_LOG"; then + echo "Error: Static analysis tool '$tool' produced zero output (missing marker: \"$marker\") in $COMPILE_LOG" MISSING_TOOLS=$((MISSING_TOOLS + 1)) fi done diff --git a/pkg/cli/compile_external_tools.go b/pkg/cli/compile_external_tools.go index 139c092264c..540e5e4572c 100644 --- a/pkg/cli/compile_external_tools.go +++ b/pkg/cli/compile_external_tools.go @@ -95,6 +95,7 @@ func RunShellcheckOnLockFiles(ctx context.Context, lockFiles []string, verbose b // from lock files and shell script resources defined in workflow frontmatter. func RunShellcheckOnLockFilesAndResources(ctx context.Context, lockFiles []string, resources []workflow.ShellScriptResource, verbose bool, strict bool) error { if len(lockFiles) == 0 && len(resources) == 0 { + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running shellcheck on run steps (0 lock files and 0 frontmatter resources found)")) compileExternalToolsLog.Printf("No shell script resources to process with shellcheck") return nil } @@ -109,9 +110,13 @@ func RunSyftOnLockFiles(lockFiles []string, verbose bool, strict bool) error { return runBatchLockFileTool("syft", lockFiles, verbose, strict, runSyftOnLockFiles) } -// runBatchLockFileTool runs a batch tool on lock files with uniform error handling +// runBatchLockFileTool runs a batch tool on lock files with uniform error handling. +// Even when there are zero lock files to process, an explicit stderr marker is +// emitted so downstream completeness checks (e.g. static-analysis-report.md) can +// distinguish "tool ran with zero input" from "tool was never invoked". func runBatchLockFileTool(toolName string, lockFiles []string, verbose bool, strict bool, runner func([]string, bool, bool) error) error { if len(lockFiles) == 0 { + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Running %s (0 lock files found)", toolName))) compileExternalToolsLog.Printf("No lock files to process with %s", toolName) return nil } diff --git a/pkg/cli/compile_external_tools_test.go b/pkg/cli/compile_external_tools_test.go index a8b1cbf6c37..0778d3a5ad5 100644 --- a/pkg/cli/compile_external_tools_test.go +++ b/pkg/cli/compile_external_tools_test.go @@ -6,6 +6,8 @@ import ( "context" "errors" "testing" + + "github.com/github/gh-aw/pkg/workflow" ) func TestHandleBatchToolErrorPreservesFatalFindingInNonStrictMode(t *testing.T) { @@ -38,10 +40,77 @@ func TestHandleBatchToolErrorPropagatesInStrictMode(t *testing.T) { } } +// TestRunBatchExternalToolsExecutesSequentialToolsWithoutEarlyAborting verifies the +// regression this PR fixes: when an early scanner (actionlint) returns an error, every +// other enabled scanner still runs to completion, in pipeline order, and the first +// error is preserved rather than being lost or causing the pipeline to abort early. func TestRunBatchExternalToolsExecutesSequentialToolsWithoutEarlyAborting(t *testing.T) { - t.Parallel() + // Not t.Parallel(): this test overrides shared package-level function variables. + + var calls []string + fakeActionlintErr := errors.New("fake actionlint finding") + + origActionlint := runBatchActionlintOnFiles + origZizmor := runBatchZizmorOnFiles + origPoutine := runBatchPoutineOnDirectory + origRunnerGuard := runBatchRunnerGuardOnDirectory + origSyft := runBatchSyftOnLockFiles + origGrype := runBatchGrypeOnLockFiles + origGrant := runBatchGrantOnLockFiles + origYamllint := runBatchYamllintOnFiles + origShellcheck := runBatchShellcheckOnLockFilesAndResources + t.Cleanup(func() { + runBatchActionlintOnFiles = origActionlint + runBatchZizmorOnFiles = origZizmor + runBatchPoutineOnDirectory = origPoutine + runBatchRunnerGuardOnDirectory = origRunnerGuard + runBatchSyftOnLockFiles = origSyft + runBatchGrypeOnLockFiles = origGrype + runBatchGrantOnLockFiles = origGrant + runBatchYamllintOnFiles = origYamllint + runBatchShellcheckOnLockFilesAndResources = origShellcheck + }) + + // The first scanner in pipeline order (actionlint) reports an error. Every + // later scanner records its invocation and returns nil so we can assert + // they all still ran, in order, after the failure. + runBatchActionlintOnFiles = func(_ context.Context, _ []string, _ bool, _ bool) error { + calls = append(calls, "actionlint") + return fakeActionlintErr + } + runBatchZizmorOnFiles = func(_ []string, _ bool, _ bool) error { + calls = append(calls, "zizmor") + return nil + } + runBatchPoutineOnDirectory = func(_ string, _ bool, _ bool) error { + calls = append(calls, "poutine") + return nil + } + runBatchRunnerGuardOnDirectory = func(_ string, _ bool, _ bool) error { + calls = append(calls, "runner-guard") + return nil + } + runBatchSyftOnLockFiles = func(_ []string, _ bool, _ bool) error { + calls = append(calls, "syft") + return nil + } + runBatchGrypeOnLockFiles = func(_ []string, _ bool, _ bool) error { + calls = append(calls, "grype") + return nil + } + runBatchGrantOnLockFiles = func(_ []string, _ bool, _ bool) error { + calls = append(calls, "grant") + return nil + } + runBatchYamllintOnFiles = func(_ []string, _ bool, _ bool) error { + calls = append(calls, "yamllint") + return nil + } + runBatchShellcheckOnLockFilesAndResources = func(_ context.Context, _ []string, _ []workflow.ShellScriptResource, _ bool, _ bool) error { + calls = append(calls, "shellcheck") + return nil + } - // Given a context and empty lock file options ctx := context.Background() config := CompileConfig{ Actionlint: true, @@ -50,27 +119,43 @@ func TestRunBatchExternalToolsExecutesSequentialToolsWithoutEarlyAborting(t *tes RunnerGuard: true, Syft: true, Grype: true, + Grant: true, Yamllint: true, + Shellcheck: true, + Strict: true, } opts := batchToolsOptions{ workflowDir: t.TempDir(), - lockFilesForActionlint: []string{}, - lockFilesForZizmor: []string{}, - lockFilesForDirTools: []string{}, - lockFilesForSyft: []string{}, - lockFilesForGrype: []string{}, - lockFilesForYamllint: []string{}, + lockFilesForActionlint: []string{"a.lock.yml"}, + lockFilesForZizmor: []string{"a.lock.yml"}, + lockFilesForDirTools: []string{"a.lock.yml"}, + lockFilesForSyft: []string{"a.lock.yml"}, + lockFilesForGrype: []string{"a.lock.yml"}, + lockFilesForGrant: []string{"a.lock.yml"}, + lockFilesForYamllint: []string{"a.lock.yml"}, + lockFilesForShellcheck: []string{"a.lock.yml"}, } stats := &CompilationStats{} var validationResults []ValidationResult strictGrantErr, batchToolErr := runBatchExternalTools(ctx, config, opts, stats, &validationResults) + if strictGrantErr != nil { t.Fatalf("expected no strictGrantErr, got %v", strictGrantErr) } - if batchToolErr != nil { - t.Fatalf("expected no batchToolErr for empty lock files, got %v", batchToolErr) + if !errors.Is(batchToolErr, fakeActionlintErr) { + t.Fatalf("expected batchToolErr to preserve the first (actionlint) error, got %v", batchToolErr) + } + + wantOrder := []string{"actionlint", "zizmor", "poutine", "runner-guard", "syft", "grype", "grant", "yamllint", "shellcheck"} + if len(calls) != len(wantOrder) { + t.Fatalf("expected all %d scanners to run despite the early actionlint error, got %d calls: %v", len(wantOrder), len(calls), calls) + } + for i, want := range wantOrder { + if calls[i] != want { + t.Fatalf("expected scanner invocation order %v, got %v (mismatch at index %d: want %q, got %q)", wantOrder, calls, i, want, calls[i]) + } } } diff --git a/pkg/cli/compile_pipeline.go b/pkg/cli/compile_pipeline.go index f661a5478c6..32f97f73b49 100644 --- a/pkg/cli/compile_pipeline.go +++ b/pkg/cli/compile_pipeline.go @@ -40,7 +40,22 @@ import ( ) var compileOrchestrationLog = logger.New("cli:compile_pipeline") -var runBatchYamllintOnFiles = RunYamllintOnFiles + +// Batch tool entry points are exposed as package-level function variables so +// tests can override them to verify the batch pipeline invokes every enabled +// scanner in order, without short-circuiting, and without depending on the +// underlying external tool binaries/Docker images being available. +var ( + runBatchActionlintOnFiles = RunActionlintOnFiles + runBatchZizmorOnFiles = RunZizmorOnFiles + runBatchPoutineOnDirectory = RunPoutineOnDirectory + runBatchRunnerGuardOnDirectory = RunRunnerGuardOnDirectory + runBatchSyftOnLockFiles = RunSyftOnLockFiles + runBatchGrypeOnLockFiles = RunGrypeOnLockFiles + runBatchGrantOnLockFiles = RunGrantOnLockFiles + runBatchYamllintOnFiles = RunYamllintOnFiles + runBatchShellcheckOnLockFilesAndResources = RunShellcheckOnLockFilesAndResources +) const fallbackCompilationErrorMessage = "compilation failed (no detailed error message available)" @@ -500,22 +515,22 @@ type batchToolsOptions struct { func runBatchLinters(ctx context.Context, config CompileConfig, opts batchToolsOptions) error { var firstErr error - if config.Actionlint && !config.NoEmit && len(opts.lockFilesForActionlint) > 0 { + if config.Actionlint && !config.NoEmit { if err := ctx.Err(); err != nil { return err } - if err := RunActionlintOnFiles(ctx, opts.lockFilesForActionlint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := runBatchActionlintOnFiles(ctx, opts.lockFilesForActionlint, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if config.Strict && firstErr == nil { firstErr = err } } } - if config.Zizmor && !config.NoEmit && len(opts.lockFilesForZizmor) > 0 { + if config.Zizmor && !config.NoEmit { if err := ctx.Err(); err != nil { return err } - if err := RunZizmorOnFiles(opts.lockFilesForZizmor, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := runBatchZizmorOnFiles(opts.lockFilesForZizmor, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if firstErr == nil { firstErr = err } @@ -528,7 +543,7 @@ func runBatchLinters(ctx context.Context, config CompileConfig, opts batchToolsO func runBatchDirScanners(ctx context.Context, config CompileConfig, opts batchToolsOptions) error { var firstErr error - if config.Poutine && !config.NoEmit && len(opts.lockFilesForDirTools) > 0 { + if config.Poutine && !config.NoEmit { if err := ctx.Err(); err != nil { return err } @@ -536,14 +551,14 @@ func runBatchDirScanners(ctx context.Context, config CompileConfig, opts batchTo if workflowDir == "" && len(opts.lockFilesForDirTools) > 0 { workflowDir = filepath.Dir(opts.lockFilesForDirTools[0]) } - if err := runBatchDirectoryTool("poutine", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, RunPoutineOnDirectory); err != nil { + if err := runBatchDirectoryTool("poutine", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, runBatchPoutineOnDirectory); err != nil { if config.Strict && firstErr == nil { firstErr = err } } } - if config.RunnerGuard && !config.NoEmit && len(opts.lockFilesForDirTools) > 0 { + if config.RunnerGuard && !config.NoEmit { if err := ctx.Err(); err != nil { return err } @@ -551,7 +566,7 @@ func runBatchDirScanners(ctx context.Context, config CompileConfig, opts batchTo if workflowDir == "" && len(opts.lockFilesForDirTools) > 0 { workflowDir = filepath.Dir(opts.lockFilesForDirTools[0]) } - if err := runBatchDirectoryTool("runner-guard", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, RunRunnerGuardOnDirectory); err != nil { + if err := runBatchDirectoryTool("runner-guard", workflowDir, config.Verbose && !config.JSONOutput, config.Strict, runBatchRunnerGuardOnDirectory); err != nil { if config.Strict && firstErr == nil { firstErr = err } @@ -568,33 +583,33 @@ func runBatchContainerScanners( stats *CompilationStats, validationResults *[]ValidationResult, ) (strictGrantErr error, containerErr error) { - if config.Syft && !config.NoEmit && len(opts.lockFilesForSyft) > 0 { + if config.Syft && !config.NoEmit { if err := ctx.Err(); err != nil { return nil, err } - if err := RunSyftOnLockFiles(opts.lockFilesForSyft, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := runBatchSyftOnLockFiles(opts.lockFilesForSyft, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if config.Strict && containerErr == nil { containerErr = err } } } - if config.Grype && !config.NoEmit && len(opts.lockFilesForGrype) > 0 { + if config.Grype && !config.NoEmit { if err := ctx.Err(); err != nil { return nil, err } - if err := RunGrypeOnLockFiles(opts.lockFilesForGrype, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := runBatchGrypeOnLockFiles(opts.lockFilesForGrype, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if config.Strict && containerErr == nil { containerErr = err } } } - if config.Grant && !config.NoEmit && len(opts.lockFilesForGrant) > 0 { + if config.Grant && !config.NoEmit { if err := ctx.Err(); err != nil { return nil, err } - if err := RunGrantOnLockFiles(opts.lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := runBatchGrantOnLockFiles(opts.lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if config.Strict { stats.Errors++ *validationResults = append(*validationResults, ValidationResult{ @@ -618,7 +633,7 @@ func runBatchContainerScanners( func runBatchScriptLinters(ctx context.Context, config CompileConfig, opts batchToolsOptions) error { var firstErr error - if config.Yamllint && !config.NoEmit && len(opts.lockFilesForYamllint) > 0 { + if config.Yamllint && !config.NoEmit { if err := ctx.Err(); err != nil { return err } @@ -629,11 +644,11 @@ func runBatchScriptLinters(ctx context.Context, config CompileConfig, opts batch } } - if config.shellcheckEnabled() && !config.NoEmit && (len(opts.lockFilesForShellcheck) > 0 || len(opts.shellcheckResources) > 0) { + if config.shellcheckEnabled() && !config.NoEmit { if err := ctx.Err(); err != nil { return err } - if err := RunShellcheckOnLockFilesAndResources(ctx, opts.lockFilesForShellcheck, opts.shellcheckResources, config.Verbose && !config.JSONOutput, config.Strict); err != nil { + if err := runBatchShellcheckOnLockFilesAndResources(ctx, opts.lockFilesForShellcheck, opts.shellcheckResources, config.Verbose && !config.JSONOutput, config.Strict); err != nil { if config.Strict && firstErr == nil { firstErr = err } diff --git a/pkg/cli/shellcheck.go b/pkg/cli/shellcheck.go index 8bbdcb79e75..46d82758e1d 100644 --- a/pkg/cli/shellcheck.go +++ b/pkg/cli/shellcheck.go @@ -143,10 +143,10 @@ func resolveDefaultShell(defaults map[string]any) string { return shell } -// extractRunStepsFromLockFile parses a compiled lock file and returns all -// run: steps whose effective shell is lintable by shellcheck. -// -// The effective shell for a step is resolved in priority order: +// extractRunStepsFromJob returns all run: steps in a single job whose +// effective shell is lintable by shellcheck. jobDefaultShell is the shell +// resolved from the job's (or workflow's) defaults.run.shell, used when a +// step does not set its own "shell" field. func extractRunStepsFromJob(job map[string]any, jobDefaultShell, lockFile string) []runStepInfo { var steps []runStepInfo rawSteps, ok := job["steps"].([]any)