Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/static-analysis-report.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 35 additions & 0 deletions .github/workflows/static-analysis-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,41 @@ 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"

# 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
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

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:
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/actionlint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion pkg/cli/compile_external_tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
123 changes: 123 additions & 0 deletions pkg/cli/compile_external_tools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
package cli

import (
"context"
"errors"
"testing"

"github.com/github/gh-aw/pkg/workflow"
)

func TestHandleBatchToolErrorPreservesFatalFindingInNonStrictMode(t *testing.T) {
Expand Down Expand Up @@ -36,3 +39,123 @@ func TestHandleBatchToolErrorPropagatesInStrictMode(t *testing.T) {
t.Fatal("expected strict mode to propagate errors, got nil")
}
}

// 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) {
// 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
}

ctx := context.Background()
config := CompileConfig{
Actionlint: true,
Zizmor: true,
Poutine: true,
RunnerGuard: true,
Syft: true,
Grype: true,
Grant: true,
Yamllint: true,
Shellcheck: true,
Strict: true,
}

opts := batchToolsOptions{
workflowDir: t.TempDir(),
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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 32f97f7: added package-level function-variable seams for all 9 batch scanner entry points and rewrote the test to make actionlint return an error, then assert all scanners still run in pipeline order and the first error is preserved. Confirmed this test fails if the pipeline is reverted to short-circuit on the first error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correction: the commit hash is ffd49a9, not 32f97f7.


if strictGrantErr != nil {
t.Fatalf("expected no strictGrantErr, got %v", strictGrantErr)
}
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])
}
}
}
Loading