Skip to content

fix: resolve three targeted lint-monster diagnostics (defer-in-loop, excess params, hardcoded path) #48520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: resolve three targeted lint-monster diagnostics (defer-in-loop, excess params, hardcoded path) #48520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb47c98
Initial plan
Copilot Jul 28, 2026
9611496
fix: address lint-monster findings - defer in loop, excess params, ha…
Copilot Jul 28, 2026
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
1 change: 0 additions & 1 deletion .github/workflows/smoke-copilot-auto.lock.yml
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion pkg/cli/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ All diagnostic output MUST go to `stderr` using `console` formatting helpers. St
| `IsDockerImageAvailable` | `func(ctx context.Context, image string) bool` | Returns true if a Docker image is present locally |
| `IsDockerImageDownloading` | `func(string) bool` | Returns true if an image pull is in progress |
| `StartDockerImageDownload` | `func(ctx context.Context, image string) (bool, func() error)` | Begins a background image pull; returns false if already pulling. The join function blocks until the goroutine exits and returns any download error. |
| `CheckAndPrepareDockerImages` | `func(ctx context.Context, useZizmor, usePoutine, useActionlint, useRunnerGuard, useSyft, useGrype, useGrant, useYamllint bool) error` | Pre-pulls security-scanner Docker images |
| `CheckAndPrepareDockerImages` | `func(ctx context.Context, opts DockerImagesOptions) error` | Pre-pulls security-scanner Docker images |
| `UpdateContainerPins` | `func(ctx, workflowDir string, verbose bool) error` | Updates container image SHA pins in workflow files |
| `CreatePRWithChanges` | `func(branchPrefix, commitMessage, prTitle, prBody string, verbose bool) (string, error)` | Creates a GitHub PR from uncommitted changes |
| `AutoMergePullRequestsCreatedAfter` | `func(repoSlug string, createdAfter time.Time, verbose bool) error` | Auto-merges eligible PRs created after a given time |
Expand Down
41 changes: 23 additions & 18 deletions pkg/cli/docker_images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,59 +251,64 @@ func StartDockerImageDownload(ctx context.Context, image string) (bool, func() e
}
}

// DockerImagesOptions specifies which Docker-based static analysis tools are requested.
type DockerImagesOptions struct {
Zizmor, Poutine, Actionlint, RunnerGuard, Syft, Grype, Grant, Yamllint bool
}

// CheckAndPrepareDockerImages checks if required Docker images are available
// for the requested static analysis tools. If any are not available, it starts
// downloading them and returns a message indicating the LLM should retry.
//
// Returns:
// - nil if all required images are available
// - error if Docker is unavailable or images are downloading/need to be downloaded
func CheckAndPrepareDockerImages(ctx context.Context, useZizmor, usePoutine, useActionlint, useRunnerGuard, useSyft, useGrype, useGrant, useYamllint bool) error {
func CheckAndPrepareDockerImages(ctx context.Context, opts DockerImagesOptions) error {

@github-actions github-actions Bot Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/codebase-design] The early-return guard manually lists every field — if a new tool is added to DockerImagesOptions, this check will silently miss it. A zero-value struct comparison is more idiomatic and self-maintaining.

💡 Suggested refactor 
// Before (manual field check — must be updated with every new field)
if !opts.Zizmor && !opts.Poutine && ... && !opts.Yamllint {
    return nil
}

// After (zero-value comparison — stays correct automatically)
if opts == (DockerImagesOptions{}) {
    return nil
}

@copilot please address this.

// If no tools requested, nothing to do
if !useZizmor && !usePoutine && !useActionlint && !useRunnerGuard && !useSyft && !useGrype && !useGrant && !useYamllint {
if !opts.Zizmor && !opts.Poutine && !opts.Actionlint && !opts.RunnerGuard && !opts.Syft && !opts.Grype && !opts.Grant && !opts.Yamllint {
return nil
}

// Check if Docker daemon is available before attempting any image operations
if !IsDockerAvailable(ctx) {
var requestedTools []string
var paramsList []string
if useZizmor {
if opts.Zizmor {
tool := "zizmor"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if usePoutine {
if opts.Poutine {
tool := "poutine"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if useActionlint {
if opts.Actionlint {
tool := "actionlint"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if useRunnerGuard {
if opts.RunnerGuard {
tool := "runner-guard"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if useSyft {
if opts.Syft {
tool := "syft"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if useGrype {
if opts.Grype {
tool := "grype"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if useGrant {
if opts.Grant {
tool := "grant"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
}
if useYamllint {
if opts.Yamllint {
tool := "yamllint"
requestedTools = append(requestedTools, tool)
paramsList = append(paramsList, tool+": false")
Expand All @@ -326,14 +331,14 @@ func CheckAndPrepareDockerImages(ctx context.Context, useZizmor, usePoutine, use
image string
name string
}{
{useZizmor, ZizmorImage, "zizmor"},
{usePoutine, PoutineImage, "poutine"},
{useActionlint, ActionlintImage, "actionlint"},
{useRunnerGuard, RunnerGuardImage, "runner-guard"},
{useSyft, SyftImage, "syft"},
{useGrype, GrypeImage, "grype"},
{useGrant, GrantImage, "grant"},
{useYamllint, YamllintImage, "yamllint"},
{opts.Zizmor, ZizmorImage, "zizmor"},
{opts.Poutine, PoutineImage, "poutine"},
{opts.Actionlint, ActionlintImage, "actionlint"},
{opts.RunnerGuard, RunnerGuardImage, "runner-guard"},
{opts.Syft, SyftImage, "syft"},
{opts.Grype, GrypeImage, "grype"},
{opts.Grant, GrantImage, "grant"},
{opts.Yamllint, YamllintImage, "yamllint"},
}

for _, img := range imagesToCheck {
Expand Down
22 changes: 11 additions & 11 deletions pkg/cli/docker_images_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestCheckAndPrepareDockerImages_NoToolsRequested(t *testing.T) {
ResetDockerPullState()

// When no tools are requested, should return nil
err := CheckAndPrepareDockerImages(context.Background(), false, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{})
if err != nil {
t.Errorf("Expected no error when no tools requested, got: %v", err)
}
Expand All @@ -31,7 +31,7 @@ func TestCheckAndPrepareDockerImages_ImageAlreadyDownloading(t *testing.T) {
SetDockerImageDownloading(ZizmorImage, true)

// Should return an error indicating to retry
err := CheckAndPrepareDockerImages(context.Background(), true, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true})
if err == nil {
t.Error("Expected error when image is downloading, got nil")
}
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestCheckAndPrepareDockerImages_MultipleImages(t *testing.T) {
SetDockerImageDownloading(PoutineImage, true)

// Request all tools
err := CheckAndPrepareDockerImages(context.Background(), true, true, true, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true, Poutine: true, Actionlint: true})
if err == nil {
t.Error("Expected error when images are downloading, got nil")
}
Expand All @@ -172,7 +172,7 @@ func TestCheckAndPrepareDockerImages_RetryMessageFormat(t *testing.T) {
// Simulate zizmor downloading
SetDockerImageDownloading(ZizmorImage, true)

err := CheckAndPrepareDockerImages(context.Background(), true, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true})
if err == nil {
t.Fatal("Expected error when image is downloading")
}
Expand Down Expand Up @@ -207,7 +207,7 @@ func TestCheckAndPrepareDockerImages_StartedDownloadingMessage(t *testing.T) {
// when the image is marked as downloading
SetDockerImageDownloading(ZizmorImage, true)

err := CheckAndPrepareDockerImages(context.Background(), true, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true})
if err == nil {
t.Fatal("Expected error when image is downloading")
}
Expand All @@ -231,7 +231,7 @@ func TestCheckAndPrepareDockerImages_ImageAlreadyAvailable(t *testing.T) {
SetMockImageAvailable(ZizmorImage, true)

// Should not return an error since the image is available
err := CheckAndPrepareDockerImages(context.Background(), true, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true})
if err != nil {
t.Errorf("Expected no error when image is available, got: %v", err)
}
Expand Down Expand Up @@ -642,7 +642,7 @@ func TestCheckAndPrepareDockerImages_DockerUnavailable(t *testing.T) {
SetMockDockerAvailable(false)

// Should return a clear error about Docker not being available
err := CheckAndPrepareDockerImages(context.Background(), true, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true})
if err == nil {
t.Fatal("Expected error when Docker is unavailable, got nil")
}
Expand Down Expand Up @@ -680,7 +680,7 @@ func TestCheckAndPrepareDockerImages_DockerUnavailable_MultipleTools(t *testing.
SetMockDockerAvailable(false)

// Request multiple tools
err := CheckAndPrepareDockerImages(context.Background(), true, false, true, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true, Actionlint: true})
if err == nil {
t.Fatal("Expected error when Docker is unavailable, got nil")
}
Expand Down Expand Up @@ -719,7 +719,7 @@ func TestCheckAndPrepareDockerImages_DockerUnavailable_NoTools(t *testing.T) {
SetMockDockerAvailable(false)

// When no tools requested, should return nil even if Docker is unavailable
err := CheckAndPrepareDockerImages(context.Background(), false, false, false, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{})
if err != nil {
t.Errorf("Expected no error when no tools requested (even with Docker unavailable), got: %v", err)
}
Expand Down Expand Up @@ -751,7 +751,7 @@ func TestCheckAndPrepareDockerImages_DockerUnavailable_ReturnsTypedError(t *test
ResetDockerPullState()
SetMockDockerAvailable(false)

err := CheckAndPrepareDockerImages(context.Background(), false, false, true, false, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Actionlint: true})
if err == nil {
t.Fatal("Expected error when Docker is unavailable, got nil")
}
Expand Down Expand Up @@ -780,7 +780,7 @@ func TestCheckAndPrepareDockerImages_RunnerGuardImageDownloading(t *testing.T) {
SetDockerImageDownloading(RunnerGuardImage, true)

// Request all tools, including runner-guard
err := CheckAndPrepareDockerImages(context.Background(), true, true, true, true, false, false, false, false)
err := CheckAndPrepareDockerImages(context.Background(), DockerImagesOptions{Zizmor: true, Poutine: true, Actionlint: true, RunnerGuard: true})
if err == nil {
t.Error("Expected error when images are downloading, got nil")
}
Expand Down
7 changes: 5 additions & 2 deletions pkg/cli/grant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import (

var grantLog = logger.New("cli:grant")

const grantPolicyFilename = ".grant.yaml"
const (
grantPolicyFilename = ".grant.yaml"
grantContainerPolicyPath = "/tmp/gh-aw-grant-policy.yaml"
)

type grantOutput struct {
Tool string `json:"tool"`
Expand Down Expand Up @@ -149,7 +152,7 @@ func grantPolicyFile() (string, error) {
}

func grantRunOnImage(imageRef, policyFile string, verbose bool) (*grantOutput, error) {
containerPolicyPath := "/tmp/gh-aw-grant-policy.yaml"
containerPolicyPath := grantContainerPolicyPath

// #nosec G204 -- imageRef and policyFile are derived from compiled lock files and the
// current repository checkout. exec.Command passes arguments directly without a shell.
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/mcp_inspect_inspector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ func spawnMCPInspector(ctx context.Context, workflowFile string, serverFilter st
// Give each process a chance to clean up
if i < len(serverProcesses)-1 {
timer := time.NewTimer(mcpProcessCleanupDelay)
defer timer.Stop()
select {
case <-timer.C:
case <-gctx.Done():
}
timer.Stop()
}
}
if err := g.Wait(); err != nil {
Expand Down
11 changes: 10 additions & 1 deletion pkg/cli/mcp_tools_readonly.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,16 @@ Returns JSON array with validation results for each workflow:
// Check if any static analysis tools are requested that require Docker images
if args.Zizmor || args.Poutine || args.Actionlint || args.RunnerGuard || args.Syft || args.Grype || args.Grant || args.Yamllint {
// Check if Docker images are available; if not, start downloading and return retry message
if err := CheckAndPrepareDockerImages(ctx, args.Zizmor, args.Poutine, args.Actionlint, args.RunnerGuard, args.Syft, args.Grype, args.Grant, args.Yamllint); err != nil {
if err := CheckAndPrepareDockerImages(ctx, DockerImagesOptions{
Zizmor: args.Zizmor,
Poutine: args.Poutine,
Actionlint: args.Actionlint,
RunnerGuard: args.RunnerGuard,
Syft: args.Syft,
Grype: args.Grype,
Grant: args.Grant,
Yamllint: args.Yamllint,
}); err != nil {
var dockerUnavailableErr *DockerUnavailableError
if errors.As(err, &dockerUnavailableErr) {
// Docker daemon is not running. Instead of failing every workflow,
Expand Down
Loading