From 0867a61014c9a3cf8becaa3157470542e4863d5b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 16:12:42 +0000 Subject: [PATCH 1/2] fix: UpdateContainerPins strips @sha256: digest from imageSet to prevent pruning all container pins Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_container_pins.go | 11 +++-- pkg/cli/update_container_pins_test.go | 71 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/pkg/cli/update_container_pins.go b/pkg/cli/update_container_pins.go index 7fc35c29fe6..1662c20c2ec 100644 --- a/pkg/cli/update_container_pins.go +++ b/pkg/cli/update_container_pins.go @@ -91,12 +91,17 @@ func UpdateContainerPins(ctx context.Context, workflowDir string, verbose bool) } } - // Build a set of images currently referenced in the compiled lock files so - // that stale entries (e.g. superseded AWF versions) can be pruned. + // Build a set of base image tags (without @sha256: digest suffix) currently + // referenced in the compiled lock files so that stale entries (e.g. superseded + // AWF versions) can be pruned. Lock files that were previously compiled may + // already embed pinned references (image:tag@sha256:...), so we strip the + // digest before comparing against container pin keys, which always use the + // base tag as the key. imageSet := make(map[string]struct { }, len(images)) for _, img := range images { - imageSet[img] = struct { + base, _, _ := strings.Cut(img, "@sha256:") + imageSet[base] = struct { }{} } diff --git a/pkg/cli/update_container_pins_test.go b/pkg/cli/update_container_pins_test.go index 45a809e87d5..39959f1b712 100644 --- a/pkg/cli/update_container_pins_test.go +++ b/pkg/cli/update_container_pins_test.go @@ -5,6 +5,7 @@ package cli import ( "os" "path/filepath" + "strings" "testing" "github.com/github/gh-aw/pkg/workflow" @@ -170,6 +171,76 @@ Manifests: } } +// TestUpdateContainerPins_PinnedLockFilesPreserveContainerPins verifies that when +// lock files already contain digest-pinned image references (image:tag@sha256:...), +// the existing container pins in actions-lock.json are NOT pruned. This is the +// regression test for the bug where gh aw update wiped out all container pins +// because collectImagesFromLockFiles returned digest-suffixed keys that did not +// match the base-tag keys used in the container pins map. +func TestUpdateContainerPins_PinnedLockFilesPreserveContainerPins(t *testing.T) { + tmpDir := t.TempDir() + + // actions-lock.json has an existing container pin using the base image tag as key. + actionsLockDir := filepath.Join(tmpDir, ".github", "aw") + require.NoError(t, os.MkdirAll(actionsLockDir, 0755)) + actionsLockContent := `{ + "entries": {}, + "containers": { + "ghcr.io/github/gh-aw-firewall/agent:0.27.9": { + "image": "ghcr.io/github/gh-aw-firewall/agent:0.27.9", + "digest": "sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269", + "pinned_image": "ghcr.io/github/gh-aw-firewall/agent:0.27.9@sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269" + } + } +} +` + actionsLockPath := filepath.Join(actionsLockDir, "actions-lock.json") + require.NoError(t, os.WriteFile(actionsLockPath, []byte(actionsLockContent), 0644)) + + // The compiled lock file already embeds the digest-pinned reference (image:tag@sha256:...). + // This is the real-world case after a prior successful gh aw update run. + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + lockFileContent := `name: test +jobs: + setup: + steps: + - name: Download container images + run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" ghcr.io/github/gh-aw-firewall/agent:0.27.9@sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269 +` + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "my-workflow.lock.yml"), []byte(lockFileContent), 0644)) + + // collectImagesFromLockFiles returns the digest-suffixed reference as-is. + images, err := collectImagesFromLockFiles(workflowsDir) + require.NoError(t, err) + require.Equal(t, []string{"ghcr.io/github/gh-aw-firewall/agent:0.27.9@sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269"}, images) + + // Build imageSet the same way UpdateContainerPins does: strip @sha256: suffix so + // the keys match the base-tag keys used in the container pins map. + imageSet := make(map[string]struct{}) + for _, img := range images { + base, _, _ := strings.Cut(img, "@sha256:") + imageSet[base] = struct{}{} + } + assert.Equal(t, map[string]struct{}{"ghcr.io/github/gh-aw-firewall/agent:0.27.9": {}}, imageSet) + + // With the correct imageSet (base tags), PruneStaleContainerPins should keep the pin. + cache := workflow.NewActionCache(tmpDir) + require.NoError(t, cache.Load()) + + pruned := cache.PruneStaleContainerPins(imageSet) + assert.Equal(t, 0, pruned, "no container pins should be pruned when lock files use pinned references") + + pin, ok := cache.GetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.9") + require.True(t, ok, "container pin should still be present") + assert.Equal(t, "sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269", pin.Digest) + + // Verify the lock file is unchanged (the existing pin covers this image). + data, err := os.ReadFile(actionsLockPath) + require.NoError(t, err) + assert.Contains(t, string(data), "ghcr.io/github/gh-aw-firewall/agent:0.27.9", "container pin should still be in actions-lock.json") +} + // TestUpdateContainerPins_PrunesStaleEntries verifies that UpdateContainerPins // removes container pin entries from actions-lock.json that are no longer // referenced in the compiled lock files (e.g. superseded AWF versions). From c79002f534f3d86c29429e35d54114e60977c1a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 20:22:54 +0000 Subject: [PATCH 2/2] refactor(test): call UpdateContainerPins end-to-end in regression test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_container_pins_test.go | 38 +++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/cli/update_container_pins_test.go b/pkg/cli/update_container_pins_test.go index 39959f1b712..96f94d14e5d 100644 --- a/pkg/cli/update_container_pins_test.go +++ b/pkg/cli/update_container_pins_test.go @@ -3,9 +3,9 @@ package cli import ( + "context" "os" "path/filepath" - "strings" "testing" "github.com/github/gh-aw/pkg/workflow" @@ -177,6 +177,9 @@ Manifests: // regression test for the bug where gh aw update wiped out all container pins // because collectImagesFromLockFiles returned digest-suffixed keys that did not // match the base-tag keys used in the container pins map. +// +// The test calls UpdateContainerPins end-to-end: because all images in the lock +// file are already digest-pinned, Docker is never invoked. func TestUpdateContainerPins_PinnedLockFilesPreserveContainerPins(t *testing.T) { tmpDir := t.TempDir() @@ -210,32 +213,29 @@ jobs: ` require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "my-workflow.lock.yml"), []byte(lockFileContent), 0644)) - // collectImagesFromLockFiles returns the digest-suffixed reference as-is. - images, err := collectImagesFromLockFiles(workflowsDir) + // UpdateContainerPins uses "." as the repo root for the action cache, so we + // chdir into tmpDir before calling it and restore the original directory after. + originalDir, _ := os.Getwd() + defer os.Chdir(originalDir) //nolint:errcheck + require.NoError(t, os.Chdir(tmpDir)) + + // Call UpdateContainerPins end-to-end. Because the lock file image is already + // digest-pinned (@sha256:...), Docker is never invoked. The function should + // prune zero pins (the bug caused it to prune all of them) and return false + // (no new pins were added). + added, err := UpdateContainerPins(context.Background(), workflowsDir, false) require.NoError(t, err) - require.Equal(t, []string{"ghcr.io/github/gh-aw-firewall/agent:0.27.9@sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269"}, images) - - // Build imageSet the same way UpdateContainerPins does: strip @sha256: suffix so - // the keys match the base-tag keys used in the container pins map. - imageSet := make(map[string]struct{}) - for _, img := range images { - base, _, _ := strings.Cut(img, "@sha256:") - imageSet[base] = struct{}{} - } - assert.Equal(t, map[string]struct{}{"ghcr.io/github/gh-aw-firewall/agent:0.27.9": {}}, imageSet) + assert.False(t, added, "no new pins should be added when all images are already pinned") - // With the correct imageSet (base tags), PruneStaleContainerPins should keep the pin. + // Reload the cache from disk and confirm the original pin is still present. cache := workflow.NewActionCache(tmpDir) require.NoError(t, cache.Load()) - pruned := cache.PruneStaleContainerPins(imageSet) - assert.Equal(t, 0, pruned, "no container pins should be pruned when lock files use pinned references") - pin, ok := cache.GetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.9") - require.True(t, ok, "container pin should still be present") + require.True(t, ok, "container pin should still be present after UpdateContainerPins") assert.Equal(t, "sha256:13f522853a688bfe24b04adbbe40b68101e8ef4b6fe0b636068527141bf1c269", pin.Digest) - // Verify the lock file is unchanged (the existing pin covers this image). + // Verify the on-disk lock file is unchanged. data, err := os.ReadFile(actionsLockPath) require.NoError(t, err) assert.Contains(t, string(data), "ghcr.io/github/gh-aw-firewall/agent:0.27.9", "container pin should still be in actions-lock.json")