Skip to content

Restore digest pinning for AWF 0.27.0 firewall sidecar images#38595

Merged
pelikhan merged 2 commits into
mainfrom
copilot/fix-digest-pinning-regression
Jun 11, 2026
Merged

Restore digest pinning for AWF 0.27.0 firewall sidecar images#38595
pelikhan merged 2 commits into
mainfrom
copilot/fix-digest-pinning-regression

Conversation

Copilot AI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Compiling workflows with gh aw v0.79.x left gh-aw-firewall/{agent,api-proxy,squid} at :0.27.0 tag-only references while other container images remained digest-pinned. This restores immutable pinning for those firewall images everywhere they are emitted in compiled lock files.

  • Pin data

    • Add the missing embedded 0.27.0 container pins for:
      • ghcr.io/github/gh-aw-firewall/agent
      • ghcr.io/github/gh-aw-firewall/api-proxy
      • ghcr.io/github/gh-aw-firewall/squid
      • ghcr.io/github/gh-aw-firewall/cli-proxy
    • Sync the updated lock data into the generated pin datasets consumed by workflow compilation.
  • Compiled output behavior

    • collectDockerImages / applyContainerPins can now resolve AWF 0.27.0 sidecars to image:tag@sha256:... using embedded pins, matching existing behavior for gh-aw-mcpg, github-mcp-server, and other pinned images.
    • The compiled lock output now carries pinned firewall refs in:
      • manifest container metadata
      • pre-download image arguments
      • AWF imageTag digest metadata
  • Regression coverage

    • Add focused tests for:
      • embedded firewall pin lookup when cache entries are absent
      • compiled workflow output for AWF v0.27.0, asserting pinned firewall refs appear in the manifest, download step, and AWF config

Example of the restored emitted form:

#   - ghcr.io/github/gh-aw-firewall/agent:0.27.0@sha256:3816d1692e6d96887b27f1e4f1d64b8d7edb43ed9d7506b8f203913cbb81c248
run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" \
  ghcr.io/github/gh-aw-firewall/agent:0.27.0@sha256:3816d1692e6d96887b27f1e4f1d64b8d7edb43ed9d7506b8f203913cbb81c248

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix digest pinning regression in gh aw v0.79.4 lock files Restore digest pinning for AWF 0.27.0 firewall sidecar images Jun 11, 2026
Copilot AI requested a review from pelikhan June 11, 2026 12:37
@pelikhan pelikhan marked this pull request as ready for review June 11, 2026 12:39
Copilot AI review requested due to automatic review settings June 11, 2026 12:39

Copilot AI left a comment

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.

Pull request overview

Restores digest pinning for ghcr.io/github/gh-aw-firewall/* sidecar images at 0.27.0 so compiled lock files emit immutable image:tag@sha256:... references (matching existing pinned-image behavior for other containers).

Changes:

  • Add missing embedded container pin entries for AWF firewall images at 0.27.0 (agent, api-proxy, squid, cli-proxy) across the pin datasets.
  • Add a unit test ensuring embedded firewall pins are used when no action-cache pins are present.
  • Add a compilation test asserting compiled lock output includes pinned firewall refs and AWF imageTag digest metadata for v0.27.0.
Show a summary per file
File Description
pkg/workflow/docker_pin_test.go Adds a unit test case validating embedded firewall pin fallback for agent:0.27.0.
pkg/workflow/docker_firewall_pin_compile_test.go Adds a compilation regression test asserting pinned firewall refs appear in the lock output for AWF v0.27.0.
pkg/workflow/data/action_pins.json Adds embedded container pins for AWF firewall images at 0.27.0 (workflow dataset).
pkg/actionpins/data/action_pins.json Adds embedded container pins for AWF firewall images at 0.27.0 (embedded actionpins dataset).
.github/aw/actions-lock.json Syncs repo lock pin dataset to include AWF firewall container pins at 0.27.0.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 1

Comment on lines +50 to +69
yamlStr := string(yaml)

expectedPins := map[string]string{
"ghcr.io/github/gh-aw-firewall/agent:0.27.0": "sha256:3816d1692e6d96887b27f1e4f1d64b8d7edb43ed9d7506b8f203913cbb81c248",
"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.0": "sha256:f28d2bd3197fb6ef9ec40ef345bbf2bb33e50151a8e72e89abb618fc3d0066eb",
"ghcr.io/github/gh-aw-firewall/squid:0.27.0": "sha256:d6a01d4cf3d928e6a7fc42e34afef228e753dce87646edc91d8a5cd0b612d9a6",
}

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)
}
}
@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer was skipped during the skills-based review.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel completed test quality analysis.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Design Decision Gate — Existing ADR Governs This Change

ADR reviewed: ADR-27762: Embedded Container Pin Fallback at Compile Time — this PR aligns with the stated decision. No new ADR required. 🏗️

The volume gate triggered (129 additions under pkg/), but the changes are entirely digest-pin data restoration (3 lock/pin JSON files) plus test coverage — there is no new architectural decision here. The change is a regression fix that restores conformance to a decision already recorded in ADR-27762.

📋 Verification Summary

ADR-27762 → Normative Spec → Embedded Pin Data Model, item 3:

Embedded container pin entries MUST be updated whenever the canonical builtin image tags resolve to new digests.

This PR satisfies that requirement directly:

  • Adds the missing 0.27.0 embedded pins for gh-aw-firewall/{agent,api-proxy,squid,cli-proxy} to both pkg/actionpins/data/action_pins.json and pkg/workflow/data/action_pins.json, plus the repo-local .github/aw/actions-lock.json cache. Each entry carries the required image, digest, and pinned_image fields (item 1).
  • Restores the MUST NOT emit a mutable tag property (Pin Resolution Order, item 2) for AWF 0.27.0 firewall sidecars, matching existing behavior for other pinned images.
  • New tests assert the embedded-pin fallback resolves 0.27.0 firewall images when the cache is absent (docker_pin_test.go) and that compiled v0.27.0 output carries pinned refs in the manifest, download step, and AWF config (docker_firewall_pin_compile_test.go) — exercising the two-level fallback the ADR specifies.

No divergence from the ADR was found. The diff contains no new abstractions, technology choices, or structural changes that would warrant a separate ADR.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · 83.8 AIC · ⌖ 8 AIC ·

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 85/100 — Excellent

Analyzed 2 test(s): 2 design, 0 implementation, 0 guideline violation(s).

📊 Metrics & Test Classification (2 tests analyzed)
Metric Value
New/modified tests analyzed 2
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (50%)
Duplicate test clusters 0
Test inflation detected No
🚨 Coding-guideline violations 0

Test Classification Details

Test File Classification Issues Detected
TestCompileWorkflow_FirewallImagesPinnedForAWF0270 pkg/workflow/docker_firewall_pin_compile_test.go:15 ✅ Design Happy-path only; bare t.Fatal(err) on write-file setup lacks context message
embedded firewall pin used when cache is absent (table row) pkg/workflow/docker_pin_test.go:38 ✅ Design Edge case covered (pins: nil fallback)

Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 2 tests — unit (//go:build !integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). Both new tests verify observable behavioral contracts of the digest-pinning fix.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §27347350229

🧪 Test quality analysis by Test Quality Sentinel · 121.4 AIC · ⌖ 19.5 AIC ·

@github-actions github-actions Bot left a comment

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.

✅ Test Quality Sentinel: 85/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%).

@github-actions github-actions Bot left a comment

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.

One non-blocking gap in the new test coverage. The data additions and the embedded-fallback unit test case look correct.

### Finding details

imageTagPart missing cli-proxy assertion (docker_firewall_pin_compile_test.go line 76)

buildAWFImageTagWithDigests (in awf_helpers.go) checks five image specs — squid, agent, agent-act, api-proxy, and cli-proxy — and appends any whose digest is available. Now that cli-proxy:0.27.0 is in the embedded pins, the compiled imageTag for v0.27.0 will be 0.27.0,squid=...,agent=...,api-proxy=...,cli-proxy=.... The test asserts the first three components but not cli-proxy, leaving that part of the fix silently unverified.

Note: cli-proxy staying out of expectedPins (the container pull-list / manifest-header check) is correct — isCliProxyNeeded is false for the test workflow.

🔎 Code quality review by PR Code Quality Reviewer · ⌖ 13.5 AIC

`0.27.0,`,
`agent=sha256:3816d1692e6d96887b27f1e4f1d64b8d7edb43ed9d7506b8f203913cbb81c248`,
`api-proxy=sha256:f28d2bd3197fb6ef9ec40ef345bbf2bb33e50151a8e72e89abb618fc3d0066eb`,
`squid=sha256:d6a01d4cf3d928e6a7fc42e34afef228e753dce87646edc91d8a5cd0b612d9a6`,

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.

Missing cli-proxy digest assertion in imageTagPart checks: buildAWFImageTagWithDigests unconditionally appends cli-proxy=sha256:... to the AWF imageTag whenever that pin is available — and cli-proxy:0.27.0 is now embedded. The generated imageTag for v0.27.0 will be 0.27.0,squid=...,agent=...,api-proxy=...,cli-proxy=..., but the test does not verify the cli-proxy component. A regression that dropped cli-proxy from the imageTag computation would go undetected.

💡 Suggested fix

Add a sixth entry to the imageTagPart slice:

`squid=sha256:d6a01d4cf3d928e6a7fc42e34afef228e753dce87646edc91d8a5cd0b612d9a6`,
`cli-proxy=sha256:42529ecb9f90da5adb00593d268dfdbd35d14bb1dc92dd897286b27ce1e3d58d`,

Note: cli-proxy intentionally stays out of expectedPins (the manifest-header / pull-list check), because isCliProxyNeeded is false for this test workflow. Only the AWF config imageTag field needs the extra assertion.

@pelikhan pelikhan merged commit 8a80afd into main Jun 11, 2026
89 of 90 checks passed
@pelikhan pelikhan deleted the copilot/fix-digest-pinning-regression branch June 11, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gh-aw-firewall/{agent,api-proxy,squid} images emitted by tag only in v0.79.x lock files (digest pinning regressed; not covered by #36579 fix)

3 participants