From cb47c9878a5f51de9b76b162aa78d3f0ec6ac6aa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:45:10 +0000 Subject: [PATCH 1/2] Initial plan From 961149650e9e1177213fb105624d16768c994bf1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:02:24 +0000 Subject: [PATCH 2/2] fix: address lint-monster findings - defer in loop, excess params, hardcoded path - mcp_inspect_inspector.go: move timer.Stop() out of defer/loop to close per iteration instead of deferring to function return - docker_images.go: introduce DockerImagesOptions struct to reduce CheckAndPrepareDockerImages from 9 to 2 parameters; update all callers and tests - grant.go: extract hard-coded '/tmp/gh-aw-grant-policy.yaml' to named constant grantContainerPolicyPath Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-copilot-auto.lock.yml | 1 - pkg/cli/README.md | 2 +- pkg/cli/docker_images.go | 41 +++++++++++-------- pkg/cli/docker_images_test.go | 22 +++++----- pkg/cli/grant.go | 7 +++- pkg/cli/mcp_inspect_inspector.go | 2 +- pkg/cli/mcp_tools_readonly.go | 11 ++++- 7 files changed, 51 insertions(+), 35 deletions(-) diff --git a/.github/workflows/smoke-copilot-auto.lock.yml b/.github/workflows/smoke-copilot-auto.lock.yml index 80f7930c2f1..9cac699d23d 100644 --- a/.github/workflows/smoke-copilot-auto.lock.yml +++ b/.github/workflows/smoke-copilot-auto.lock.yml @@ -149,7 +149,6 @@ jobs: GH_AW_INFO_FIREWALL_TYPE: "squid" GH_AW_INFO_FRONTMATTER_EMOJI: "🌸" GH_AW_COMPILED_STRICT: "true" - GH_AW_INFO_MODEL_COSTS: '{"providers":{"github-copilot":{"models":{"auto":{"cost":{"input":"8.5e-07","output":"1.55e-06"}}}}}}' GH_AW_INFO_FEATURES: '{"gh-aw-detection":false}' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: diff --git a/pkg/cli/README.md b/pkg/cli/README.md index 1613dbc1a2c..e18b789cb85 100644 --- a/pkg/cli/README.md +++ b/pkg/cli/README.md @@ -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 | diff --git a/pkg/cli/docker_images.go b/pkg/cli/docker_images.go index cecb0e87678..afbf869a838 100644 --- a/pkg/cli/docker_images.go +++ b/pkg/cli/docker_images.go @@ -251,6 +251,11 @@ 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. @@ -258,9 +263,9 @@ func StartDockerImageDownload(ctx context.Context, image string) (bool, func() e // 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 { // 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 } @@ -268,42 +273,42 @@ func CheckAndPrepareDockerImages(ctx context.Context, useZizmor, usePoutine, use 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") @@ -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 { diff --git a/pkg/cli/docker_images_test.go b/pkg/cli/docker_images_test.go index d8d42ab13d3..f01f5133d74 100644 --- a/pkg/cli/docker_images_test.go +++ b/pkg/cli/docker_images_test.go @@ -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) } @@ -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") } @@ -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") } @@ -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") } @@ -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") } @@ -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) } @@ -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") } @@ -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") } @@ -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) } @@ -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") } @@ -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") } diff --git a/pkg/cli/grant.go b/pkg/cli/grant.go index b28c28b02cb..4f919e8005a 100644 --- a/pkg/cli/grant.go +++ b/pkg/cli/grant.go @@ -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"` @@ -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. diff --git a/pkg/cli/mcp_inspect_inspector.go b/pkg/cli/mcp_inspect_inspector.go index 33324d654f7..1f1f0c2ca7c 100644 --- a/pkg/cli/mcp_inspect_inspector.go +++ b/pkg/cli/mcp_inspect_inspector.go @@ -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 { diff --git a/pkg/cli/mcp_tools_readonly.go b/pkg/cli/mcp_tools_readonly.go index 1e13f8e3a22..c168c67a1b6 100644 --- a/pkg/cli/mcp_tools_readonly.go +++ b/pkg/cli/mcp_tools_readonly.go @@ -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,