Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/changeset.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/daily-cache-strategy-analyzer.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/daily-fact.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/daily-observability-report.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/smoke-call-workflow.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions .github/workflows/smoke-claude-on-copilot.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .github/workflows/smoke-codex.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions pkg/workflow/threat_detection.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,13 @@ func (c *Compiler) buildDetectionJobSteps(data *WorkflowData) []string {
// Step 1: Pull AWF container images - the detection engine runs inside AWF (firewall),
// so pre-pulling the containers speeds up execution and avoids on-demand pulls.
//
// For Codex detection, MCP setup generation already emits this step, so skip here
// to avoid duplicate step IDs/names in the detection job.
if c.getThreatDetectionEngineID(data) != "codex" {
// For the inline Codex detection path (gh-aw-detection feature disabled), MCP setup
// generation already emits this step via generateDownloadDockerImagesStep, so skip here
// to avoid duplicate step names in the detection job.
// For the external detector path (gh-aw-detection: true), MCP setup is not called for
// the detection job, so the download step must be emitted here unconditionally.
usingExternalDetector := isFeatureEnabled(constants.GHAWDetectionFeatureFlag, data)

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.

Nice—deriving usingExternalDetector from the feature flag makes the codex-skip condition explicit and self-documenting.

if c.getThreatDetectionEngineID(data) != "codex" || usingExternalDetector {

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.

Good defensive condition: != "codex" || usingExternalDetector correctly covers the external-detector path that bypasses MCP setup.

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.

Smoke test: Clear comment explaining the external detector path. 👍

steps = append(steps, c.buildPullAWFContainersStep(data)...)
}

Expand Down
61 changes: 61 additions & 0 deletions pkg/workflow/threat_detection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1835,6 +1835,67 @@ func TestCleanFirewallDirsStepOrdering(t *testing.T) {
}
}

func TestBuildDetectionJobStepsCodexExternalDetectorIncludesContainerDownload(t *testing.T) {
// Regression test: when engine=codex and gh-aw-detection feature is enabled (external
// detector path), the detection job must include a "Download container images" step.
// Previously the step was omitted under the incorrect assumption that MCP setup generation
// would emit it — MCP setup is only called for the inline codex detection path.
compiler := NewCompiler()

t.Run("codex with gh-aw-detection includes Download container images", func(t *testing.T) {
data := &WorkflowData{
AI: "codex",
SafeOutputs: &SafeOutputsConfig{
ThreatDetection: &ThreatDetectionConfig{},
},
Features: map[string]any{
string(constants.GHAWDetectionFeatureFlag): true,
},
SandboxConfig: &SandboxConfig{
Agent: &AgentSandboxConfig{
Type: SandboxTypeAWF,
},
},
}

steps := compiler.buildDetectionJobSteps(data)
joined := strings.Join(steps, "")

if !strings.Contains(joined, "Download container images") {
t.Errorf("expected 'Download container images' step in codex external detector detection job steps\ngot:\n%s", joined)
}
if !strings.Contains(joined, "download_docker_images.sh") {
t.Errorf("expected 'download_docker_images.sh' in detection job steps\ngot:\n%s", joined)
}
Comment on lines +1864 to +1869
})

t.Run("codex without gh-aw-detection emits exactly one container download (inline path via MCP setup)", func(t *testing.T) {
data := &WorkflowData{
AI: "codex",
SafeOutputs: &SafeOutputsConfig{
ThreatDetection: &ThreatDetectionConfig{},
},
Features: map[string]any{},
SandboxConfig: &SandboxConfig{
Agent: &AgentSandboxConfig{
Type: SandboxTypeAWF,
},
},
}

steps := compiler.buildDetectionJobSteps(data)
joined := strings.Join(steps, "")

// For the inline codex path, MCP setup generation (inside buildDetectionEngineExecutionStep)
// emits the "Download container images" step exactly once. buildPullAWFContainersStep must
// NOT also emit it, or the step would appear twice and trip duplicate-step validation.
downloadCount := strings.Count(joined, "Download container images")
if downloadCount != 1 {
t.Errorf("expected exactly one 'Download container images' step for inline codex path, got %d\n%s", downloadCount, joined)
}
})
Comment on lines +1872 to +1896
}

func TestBuildPullAWFContainersStepPropagatesFeatures(t *testing.T) {
compiler := NewCompiler()

Expand Down
Loading