Honor container image overrides in threat-detection jobs

docs/adr/63014-honor-detection-job-container-overrides.md

@@ -0,0 +1,45 @@
+# ADR-63014: Honor detection job container overrides
+
+**Date**: 2026-09-23
+**Status**: Draft
+**Deciders**: gh-aw maintainers
+
+---
+
+### Context
+
+Threat-detection jobs are derived from the parent workflow configuration, but they previously dropped `sandbox.agent.images` and `container_pins` while building the detection-specific workflow data. The PR description and diff show that this caused detection jobs to fall back to the default `ghcr.io` AWF registry even when the main agent workflow had been configured to use mirrored or private container images. This created inconsistent runtime behavior between the main workflow and threat-detection jobs and could break repositories that require private-registry routing or pinned mirrored images. The change is focused on how detection-job workflow data inherits container-related configuration from the parent workflow.
+
+### Decision
+
+We will propagate `sandbox.agent.images` and `container_pins` into threat-detection workflow data and clone both maps before assigning them to the derived detection configuration. When `container_pins` redirects a required default AWF image, the detection job will translate the mappings into a complete `container.images` runtime manifest. Threat-detection jobs will therefore use the same image-role overrides and repository container-pin mappings as the parent workflow during both pre-pull and runtime execution. We chose this because the PR evidence shows the architectural intent is configuration consistency across workflow variants, while cloning prevents the derived detection configuration from mutating the parent workflow state.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep threat-detection jobs on the default AWF registry
+
+This preserves the previous behavior and avoids carrying more configuration into derived detection workflow data. It was not chosen because the PR shows that defaulting to `ghcr.io` breaks expected parity with the parent workflow and ignores explicit private-registry or mirrored-image configuration already supplied by repository authors.
+
+#### Alternative 2: Propagate overrides by sharing the parent maps directly
+
+This would let detection jobs see the same image and pin data with less copying logic. It was not chosen because the tests in this PR explicitly validate that detection workflow data must not alias the parent maps; sharing references would make later mutation in derived workflow construction able to leak back into the parent configuration.
+
+### Consequences
+
+#### Positive
+- Threat-detection jobs now honor the same mirrored or private container image configuration as the primary workflow jobs.
+- Detection pre-pull steps use mapped container references instead of silently falling back to the default AWF registry.
+- Detection runtime configuration uses mapped references instead of starting default-registry images.
+- Cloning the image and pin mappings preserves isolation between parent workflow data and derived detection workflow data.
+
+#### Negative
+- The threat-detection workflow builder now has extra configuration-copying logic that must stay synchronized with future sandbox configuration changes.
+- Additional tests and cloning introduce a small maintenance cost whenever container override behavior evolves.
+
+#### Neutral
+- The change does not introduce a new registry-selection feature; it extends existing override mechanisms into detection jobs.
+- Runtime behavior only changes for workflows that already set `sandbox.agent.images` or `container_pins`; workflows using defaults continue to behave the same.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/workflow/sandbox_agent_images.go

@@ -6,10 +6,10 @@
 // by the enabled feature set must be listed and AWF fails closed instead of falling
 // back to the official registry.
 //
-// The repository-level .github/workflows/aw.json "container_pins" setting can
-// redirect default AWF references for predownload and lock metadata, but it
-// cannot change the role references AWF resolves at runtime. This manifest is
-// authoritative for both runtime role selection and gh-aw's matching predownload.
+// The repository-level .github/workflows/aw.json "container_pins" setting redirects
+// default AWF references for predownload and lock metadata. Detection jobs also
+// translate those redirects into a closed runtime manifest. An explicitly configured
+// manifest remains authoritative for runtime role selection and matching predownload.
 
 package workflow
 

pkg/workflow/threat_detection_external.go

@@ -3,6 +3,7 @@ package workflow
 
 import (
 	"fmt"
+	"maps"
 	"path"
 	"slices"
 	"strconv"
@@ -115,21 +116,23 @@ func buildThreatDetectionWorkflowData(data *WorkflowData, engineID string) *Work
 	}
 
 	detectionData := &WorkflowData{
-		AI:                engineID,
-		ActionCache:       data.ActionCache,
-		Features:          data.Features,
-		Jobs:              data.Jobs,
-		Permissions:       data.Permissions,
-		ParsedFrontmatter: data.ParsedFrontmatter,
-		CachedPermissions: data.CachedPermissions,
-		ModelCosts:        data.ModelCosts,
-		IsDetectionRun:    true,
-		RunnerConfig:      data.RunnerConfig,
-		TimeoutMinutes:    "timeout-minutes: " + resolveDetectionJobTimeoutValue(data),
-		CompiledVersion:   data.CompiledVersion,
+		AI:                   engineID,
+		ActionCache:          data.ActionCache,
+		Features:             data.Features,
+		Jobs:                 data.Jobs,
+		Permissions:          data.Permissions,
+		ParsedFrontmatter:    data.ParsedFrontmatter,
+		CachedPermissions:    data.CachedPermissions,
+		ModelCosts:           data.ModelCosts,
+		IsDetectionRun:       true,
+		RunnerConfig:         data.RunnerConfig,
+		TimeoutMinutes:       "timeout-minutes: " + resolveDetectionJobTimeoutValue(data),
+		CompiledVersion:      data.CompiledVersion,
+		ContainerPinMappings: maps.Clone(data.ContainerPinMappings),
 		SandboxConfig: &SandboxConfig{
 			Agent: &AgentSandboxConfig{
-				Type: SandboxTypeAWF,
+				Type:   SandboxTypeAWF,
+				Images: maps.Clone(getSandboxAgentImages(data)),
 			},
 		},
 	}
@@ -145,10 +148,42 @@ func buildThreatDetectionWorkflowData(data *WorkflowData, engineID string) *Work
 		}
 		detectionData.SandboxConfig.Agent.Version = firewallCopy.Version
 	}
+	if len(detectionData.SandboxConfig.Agent.Images) == 0 {
+		detectionData.SandboxConfig.Agent.Images = buildThreatDetectionContainerImages(detectionData)
+	}
 
 	return detectionData
 }
 
+// buildThreatDetectionContainerImages translates AWF container pin redirects into
+// the closed runtime manifest expected by AWF. A complete manifest is emitted only
+// when at least one required AWF role is redirected.
+func buildThreatDetectionContainerImages(data *WorkflowData) map[string]string {
+	if len(data.ContainerPinMappings) == 0 {
+		return nil
+	}
+
+	imageTag := getAWFImageTag(getFirewallConfig(data))
+	images := make(map[string]string)
+	hasMappedImage := false
+	for _, role := range requiredAWFImageRoles(data) {
+		image := defaultAWFImageForRole(role, imageTag)
+		if applyContainerPinMappingFromData(image, data) != image {
+			hasMappedImage = true
+		}
+		resolved := resolveContainerImage(image, data)
+		if !awfPinnedImagePattern.MatchString(resolved) {
+			threatLog.Printf("Skipping detection container manifest: %s image is not digest-pinned", role)
+			return nil
+		}
+		images[role] = resolved
+	}
+	if !hasMappedImage {
+		return nil
+	}
+	return images
+}
+
 // buildPullAWFContainersStep creates a step that pre-pulls AWF (agent workflow firewall)
 // container images in the detection job. The detection engine runs inside AWF, which uses
 // the firewall stack containers needed for the selected topology (squid, agent, api-proxy,

pkg/workflow/threat_detection_job_runtime_test.go

@@ -3,6 +3,7 @@
 package workflow
 
 import (
+	"encoding/json"
 	"strings"
 	"testing"
 
@@ -247,6 +248,168 @@ func TestBuildPullAWFContainersStepPropagatesFeatures(t *testing.T) {
 	})
 }
 
+func TestBuildThreatDetectionWorkflowDataPropagatesImageOverrides(t *testing.T) {
+	images := map[string]string{
+		awfImageRoleSquid:    "mirror.example.com/awf/squid:v1@sha256:" + strings.Repeat("a", 64),
+		awfImageRoleAgent:    "mirror.example.com/awf/agent:v1@sha256:" + strings.Repeat("b", 64),
+		awfImageRoleAPIProxy: "mirror.example.com/awf/api-proxy:v1@sha256:" + strings.Repeat("c", 64),
+	}
+	containerPins := map[string]string{
+		"ghcr.io/github/gh-aw-firewall/squid:v1": images[awfImageRoleSquid],
+	}
+	data := &WorkflowData{
+		AI: "copilot",
+		SandboxConfig: &SandboxConfig{
+			Agent: &AgentSandboxConfig{
+				Type:   SandboxTypeAWF,
+				Images: images,
+			},
+		},
+		ContainerPinMappings: containerPins,
+	}
+
+	detectionData := buildThreatDetectionWorkflowData(data, "")
+
+	for role, image := range images {
+		if detectionData.SandboxConfig.Agent.Images[role] != image {
+			t.Errorf("detection image %q = %q, want %q", role, detectionData.SandboxConfig.Agent.Images[role], image)
+		}
+	}
+	for source, target := range containerPins {
+		if detectionData.ContainerPinMappings[source] != target {
+			t.Errorf("detection container pin %q = %q, want %q", source, detectionData.ContainerPinMappings[source], target)
+		}
+	}
+
+	detectionData.SandboxConfig.Agent.Images[awfImageRoleSquid] = "changed"
+	detectionData.ContainerPinMappings["ghcr.io/github/gh-aw-firewall/squid:v1"] = "changed"
+	if images[awfImageRoleSquid] == "changed" {
+		t.Error("detection image overrides must not alias the parent map")
+	}
+	if containerPins["ghcr.io/github/gh-aw-firewall/squid:v1"] == "changed" {
+		t.Error("detection container pin mappings must not alias the parent map")
+	}
+}
+
+func TestBuildPullAWFContainersStepUsesSandboxAgentImages(t *testing.T) {
+	compiler := NewCompiler()
+	mirrorRegistry := "mirror.example.com/awf/"
+	data := &WorkflowData{
+		AI: "copilot",
+		SandboxConfig: &SandboxConfig{
+			Agent: &AgentSandboxConfig{
+				Type: SandboxTypeAWF,
+				Images: map[string]string{
+					awfImageRoleSquid:    mirrorRegistry + "squid:v1@sha256:" + strings.Repeat("a", 64),
+					awfImageRoleAgent:    mirrorRegistry + "agent:v1@sha256:" + strings.Repeat("b", 64),
+					awfImageRoleAPIProxy: mirrorRegistry + "api-proxy:v1@sha256:" + strings.Repeat("c", 64),
+				},
+			},
+		},
+	}
+
+	steps := strings.Join(compiler.buildPullAWFContainersStep(data), "")
+
+	if !strings.Contains(steps, mirrorRegistry) {
+		t.Errorf("expected detection pull step to use sandbox.agent.images overrides; got:\n%s", steps)
+	}
+	if strings.Contains(steps, constants.DefaultFirewallRegistry) {
+		t.Errorf("expected detection pull step not to use default AWF registry; got:\n%s", steps)
+	}
+}
+
+func TestBuildPullAWFContainersStepUsesContainerPinMappings(t *testing.T) {
+	compiler := NewCompiler()
+	source := constants.DefaultFirewallRegistry + "/squid:1.2.3"
+	target := "mirror.example.com/awf/squid:1.2.3@sha256:" + strings.Repeat("a", 64)
+	data := &WorkflowData{
+		AI: "copilot",
+		SandboxConfig: &SandboxConfig{
+			Agent: &AgentSandboxConfig{
+				Type:    SandboxTypeAWF,
+				Version: "v1.2.3",
+			},
+		},
+		ContainerPinMappings: map[string]string{
+			source: target,
+		},
+	}
+
+	steps := strings.Join(compiler.buildPullAWFContainersStep(data), "")
+
+	if !strings.Contains(steps, target) {
+		t.Errorf("expected detection pull step to use container_pins mapping %q; got:\n%s", target, steps)
+	}
+	if strings.Contains(steps, source) {
+		t.Errorf("expected detection pull step not to use mapped source image %q; got:\n%s", source, steps)
+	}
+}
+
+func TestBuildThreatDetectionAWFConfigUsesContainerPinMappings(t *testing.T) {
+	const version = "1.2.3"
+	containerPins := map[string]string{}
+	expectedImages := map[string]string{}
+	for i, role := range []string{awfImageRoleSquid, awfImageRoleAgent, awfImageRoleAPIProxy} {
+		source := defaultAWFImageForRole(role, version)
+		target := "mirror.example.com/awf/" + strings.TrimPrefix(source, constants.DefaultFirewallRegistry+"/") +
+			"@sha256:" + strings.Repeat(string(rune('a'+i)), 64)
+		containerPins[source] = target
+		expectedImages[role] = target
+	}
+	data := &WorkflowData{
+		AI: "copilot",
+		NetworkPermissions: &NetworkPermissions{
+			Firewall: &FirewallConfig{Enabled: true, Version: version},
+		},
+		ContainerPinMappings: containerPins,
+	}
+
+	configJSON, err := BuildAWFConfigJSON(AWFCommandConfig{
+		EngineName:     "copilot",
+		AllowedDomains: "github.com",
+		WorkflowData:   buildThreatDetectionWorkflowData(data, ""),
+	})
+	if err != nil {
+		t.Fatalf("BuildAWFConfigJSON() error = %v", err)
+	}
+
+	var config struct {
+		Container AWFContainerConfig `json:"container"`
+	}
+	if err := json.Unmarshal([]byte(configJSON), &config); err != nil {
+		t.Fatalf("unmarshal AWF config: %v", err)
+	}
+	if config.Container.ImageTag != "" {
+		t.Errorf("detection imageTag = %q, want empty when mapped images are emitted", config.Container.ImageTag)
+	}
+	for role, target := range expectedImages {
+		if config.Container.Images[role] != target {
+			t.Errorf("detection runtime image %q = %q, want %q", role, config.Container.Images[role], target)
+		}
+	}
+	if strings.Contains(configJSON, constants.DefaultFirewallRegistry) {
+		t.Errorf("detection AWF config must not use the default registry when all required roles are mapped: %s", configJSON)
+	}
+}
+
+func TestBuildThreatDetectionContainerImagesRequiresPinnedOptionalRoles(t *testing.T) {
+	const version = "1.2.3"
+	data := &WorkflowData{
+		RunnerConfig: &RunnerConfig{Topology: RunnerTopologyArcDind},
+		NetworkPermissions: &NetworkPermissions{
+			Firewall: &FirewallConfig{Enabled: true, Version: version},
+		},
+		ContainerPinMappings: map[string]string{
+			defaultAWFImageForRole(awfImageRoleSquid, version): "mirror.example.com/awf/squid:" + version +
+				"@sha256:" + strings.Repeat("a", 64),
+		},
+	}
+
+	if images := buildThreatDetectionContainerImages(data); images != nil {
+		t.Errorf("buildThreatDetectionContainerImages() = %v, want nil for unpinned buildTools role", images)
+	}
+}
+
 func TestBuildPullAWFContainersStepPropagatesRunnerTopology(t *testing.T) {
 	compiler := NewCompiler()
 	buildToolsImagePrefix := constants.DefaultFirewallRegistry + "/build-tools:"