diff --git a/docs/adr/43343-regression-tests-for-firewall-digest-pinning.md b/docs/adr/43343-regression-tests-for-firewall-digest-pinning.md new file mode 100644 index 00000000000..ff1609a5d62 --- /dev/null +++ b/docs/adr/43343-regression-tests-for-firewall-digest-pinning.md @@ -0,0 +1,49 @@ +# ADR-43343: Two-Layer Regression Tests for gh-aw-firewall Digest Pinning + +**Date**: 2026-07-04 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +Consumer-compiled lock files produced by gh-aw v0.82.2 emitted all four `gh-aw-firewall` images (`agent`, `api-proxy`, `cli-proxy`, `squid`) with tag-only references — no `digest` or `pinned_image` field. The embedded fallback pins in `action_pins.json` were already correct for the default version (`0.27.22`), but the existing regression tests only covered the older `0.27.0` version and did not cover `cli-proxy` (introduced in v0.82) at all. This coverage gap meant the regression shipped silently: no test failed, yet consumers received unpinned images. Without a test guard, any future `DefaultFirewallVersion` bump or sidecar addition carries the same silent-regression risk. + +### Decision + +We will add two regression tests tied to `constants.DefaultFirewallVersion` that together verify end-to-end digest pinning for all current firewall sidecars: + +1. **Unit test** (`TestApplyContainerPins_DefaultFirewallVersion` in `docker_pin_test.go`) — asserts that each of the four images has a valid entry in the embedded pin table (non-empty `Digest` and `PinnedImage`), confirming `action_pins.json` coverage. +2. **End-to-end compile test** (`TestCompileWorkflow_FirewallImagesPinnedForDefaultVersion` in `docker_firewall_pin_compile_test.go`) — compiles a minimal workflow with `tools.github.mode: gh-proxy` (so `cli-proxy` is included) and asserts that the resulting lock file contains full `image`/`digest`/`pinned_image` metadata and correctly encoded `--image-tag` AWF config for all four images. + +By binding both tests to `constants.DefaultFirewallVersion`, any version bump that does not refresh `action_pins.json` will immediately cause CI failures rather than silently shipping unpinned images. + +### Alternatives Considered + +#### Alternative 1: Fix action_pins.json without adding tests + +Correct the embedded pin data for `0.27.22` and `cli-proxy` without introducing new tests. This removes the immediate regression but leaves the root cause — absence of a test guard — unaddressed. The next `DefaultFirewallVersion` bump or new sidecar addition would carry the same silent-regression risk. Rejected because it treats the symptom rather than the systemic gap. + +#### Alternative 2: Unit-only test (skip the compile test) + +Add the pin-table unit test (`TestApplyContainerPins_DefaultFirewallVersion`) but omit the compile test. This would catch missing `action_pins.json` entries but would not detect integration-level failures where the pin table is populated yet the compiler fails to emit `digest`/`pinned_image` fields into the lock file. The compile test provides a separate failure mode that the unit test cannot cover. Rejected because the historical regression was an integration-level failure (correct pin data, incorrect lock-file output), which only an end-to-end compile test would have caught. + +### Consequences + +#### Positive +- Future `DefaultFirewallVersion` bumps will fail CI immediately if `action_pins.json` is not updated with correct digests for all sidecars. +- The `cli-proxy` sidecar (new in v0.82) is now explicitly covered alongside the three legacy images, closing the coverage gap that allowed the v0.82.2 regression. +- Both tests use `constants.DefaultFirewallVersion` rather than a hardcoded version string, so they automatically track the canonical version constant. + +#### Negative +- The compile test hardcodes specific digest SHA-256 values for the `0.27.22` release; when firewall images are rebuilt (e.g., base-image security patches), the test expected values must be updated even if the logic is unchanged. +- Two test layers (unit + compile) increase the test maintenance surface. Every time the compiler's lock-file schema changes, both tests may need updating. + +#### Neutral +- The compile test requires `tools.github.mode: gh-proxy` in the workflow frontmatter; this is a test-only detail that does not affect production behavior. +- No production source files were changed in this PR — the change is confined to `_test.go` files, so there is no runtime impact on consumers. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/workflow/docker_firewall_pin_compile_test.go b/pkg/workflow/docker_firewall_pin_compile_test.go index 9fe014a6332..7cac9e8aa6b 100644 --- a/pkg/workflow/docker_firewall_pin_compile_test.go +++ b/pkg/workflow/docker_firewall_pin_compile_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/stringutil" "github.com/github/gh-aw/pkg/testutil" ) @@ -80,3 +81,81 @@ Test workflow.` } } } + +// TestCompileWorkflow_FirewallImagesPinnedForDefaultVersion is a regression test for +// gh-aw#43307: the four gh-aw-firewall images at the current default version +// (constants.DefaultFirewallVersion) must all be digest-pinned in consumer lock files +// even when no local action-cache is present. This covers the cli-proxy image +// introduced in v0.82 as well as the three legacy images (agent, api-proxy, squid). +func TestCompileWorkflow_FirewallImagesPinnedForDefaultVersion(t *testing.T) { + // Strip the leading "v" to get the Docker image tag (mirrors getAWFImageTag). + imageTag := strings.TrimPrefix(string(constants.DefaultFirewallVersion), "v") + + // Enable tools.github.mode=gh-proxy so that the cli-proxy sidecar container is + // included in the Docker pull list and therefore also pinned in the lock file. + frontmatter := `--- +on: workflow_dispatch +engine: claude +network: + allowed: + - defaults +tools: + github: + mode: gh-proxy +--- + +# Test +Test workflow.` + + tmpDir := testutil.TempDir(t, "docker-firewall-pins-default-version-test") + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(frontmatter), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockFile := stringutil.MarkdownToLockFile(testFile) + yaml, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + yamlStr := string(yaml) + + expectedPins := map[string]string{ + "ghcr.io/github/gh-aw-firewall/agent:" + imageTag: "sha256:55f06588411008b7148eb64b8dfe28602a0cce3675b36c6b190b54aca138468e", + "ghcr.io/github/gh-aw-firewall/api-proxy:" + imageTag: "sha256:afb9ff9140b17d38871dfb9dbac5ff8689ea634c2f91c435da2825192d4881c1", + "ghcr.io/github/gh-aw-firewall/cli-proxy:" + imageTag: "sha256:e23e1604241f579b418e6522d938285b57ada31bc27742a65c90ee2250b1755c", + "ghcr.io/github/gh-aw-firewall/squid:" + imageTag: "sha256:3cdcc1e2b4b4fe602ba69fd3e21aac7ac512d5c1fce24df4ce69dc4f98164b59", + } + + for image, digest := range expectedPins { + pinnedImage := image + "@" + digest + if !strings.Contains(yamlStr, `"image":"`+image+`","digest":"`+digest+`","pinned_image":"`+pinnedImage+`"`) { + t.Errorf("Expected manifest header to include pinned metadata for %s", image) + } + if !strings.Contains(yamlStr, "# - "+pinnedImage) { + t.Errorf("Expected pinned container comment for %s", image) + } + if !strings.Contains(yamlStr, pinnedImage) { + t.Errorf("Expected pinned download reference for %s", image) + } + } + + for _, imageTagPart := range []string{ + `imageTag`, + imageTag + `,`, + `agent=sha256:55f06588411008b7148eb64b8dfe28602a0cce3675b36c6b190b54aca138468e`, + `api-proxy=sha256:afb9ff9140b17d38871dfb9dbac5ff8689ea634c2f91c435da2825192d4881c1`, + `cli-proxy=sha256:e23e1604241f579b418e6522d938285b57ada31bc27742a65c90ee2250b1755c`, + `squid=sha256:3cdcc1e2b4b4fe602ba69fd3e21aac7ac512d5c1fce24df4ce69dc4f98164b59`, + } { + if !strings.Contains(yamlStr, imageTagPart) { + t.Errorf("Expected AWF config JSON to include %s", imageTagPart) + } + } +} diff --git a/pkg/workflow/docker_pin_test.go b/pkg/workflow/docker_pin_test.go index d1eee3967d7..3432c10aef6 100644 --- a/pkg/workflow/docker_pin_test.go +++ b/pkg/workflow/docker_pin_test.go @@ -3,6 +3,7 @@ package workflow import ( + "strings" "testing" "github.com/github/gh-aw/pkg/constants" @@ -105,6 +106,31 @@ func TestApplyContainerPins(t *testing.T) { } } +// TestApplyContainerPins_DefaultFirewallVersion is a regression test for gh-aw#43307: +// all four gh-aw-firewall images at constants.DefaultFirewallVersion (including cli-proxy, +// which was new in v0.82) must have entries in the embedded pin table so that consumer +// compiles without a local cache still emit digest-pinned references. +// Using constants means the test automatically tracks version bumps. +func TestApplyContainerPins_DefaultFirewallVersion(t *testing.T) { + imageTag := strings.TrimPrefix(string(constants.DefaultFirewallVersion), "v") + sidecars := []string{"agent", "api-proxy", "cli-proxy", "squid"} + + for _, sidecar := range sidecars { + image := constants.DefaultFirewallRegistry + "/" + sidecar + ":" + imageTag + t.Run(sidecar, func(t *testing.T) { + pin, ok := getEmbeddedContainerPin(image) + require.True(t, ok, "embedded pin must exist for %s", image) + require.NotEmpty(t, pin.Digest, "Digest must be non-empty for %s", image) + require.NotEmpty(t, pin.PinnedImage, "PinnedImage must be non-empty for %s", image) + + refs, pinEntries := applyContainerPins([]string{image}, nil) + require.Len(t, refs, 1) + assert.Equal(t, pin.PinnedImage, refs[0], "resolved ref for %s", image) + assert.Equal(t, pin.Digest, pinEntries[0].Digest, "digest in manifest entry for %s", image) + }) + } +} + // TestCollectDockerImages_StoresInWorkflowData verifies that collectDockerImages // populates workflowData.DockerImages and DockerImagePins with the collected image refs. func TestCollectDockerImages_StoresInWorkflowData(t *testing.T) {