From 60ae32d302622c0363645fe7fbaf474750a1c292 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:28:16 +0000 Subject: [PATCH 1/5] Initial plan From 8bf9a5402b51a825fba9afafc23e23b24ef2ba40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:02:58 +0000 Subject: [PATCH 2/5] Fix threat detection custom API target/env inheritance Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/threat_detection_external.go | 15 +++++- pkg/workflow/threat_detection_helpers.go | 26 ++++++++- pkg/workflow/threat_detection_test.go | 64 +++++++++++++++++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/threat_detection_external.go b/pkg/workflow/threat_detection_external.go index 5ce1301a706..108ab0a38cf 100644 --- a/pkg/workflow/threat_detection_external.go +++ b/pkg/workflow/threat_detection_external.go @@ -220,6 +220,7 @@ func (c *Compiler) buildInstallDetectionEngineForExternalDetectorStep(data *Work CopilotSDK: ec.CopilotSDK, } } + threatDetectionData.EngineConfig.Env = mergeThreatDetectionEngineEnv(data, threatDetectionData.EngineConfig.Env) if threatDetectionData.EngineConfig.APITarget == "" && data.EngineConfig != nil { threatDetectionData.EngineConfig.APITarget = data.EngineConfig.APITarget } @@ -294,10 +295,20 @@ func (c *Compiler) buildExternalDetectorExecutionStep(data *WorkflowData) []stri if canReuseThreatDetectionEngineConfigForExternalDetector(data, engineID) { ec := data.SafeOutputs.ThreatDetection.EngineConfig threatDetectionData.EngineConfig = &EngineConfig{ - ID: engineID, - APITarget: ec.APITarget, + ID: engineID, + Model: ec.Model, + Version: ec.Version, + Env: ec.Env, + Config: ec.Config, + Args: ec.Args, + Command: ec.Command, + APITarget: ec.APITarget, + HarnessScript: ec.HarnessScript, + Driver: ec.Driver, + CopilotSDK: ec.CopilotSDK, } } + threatDetectionData.EngineConfig.Env = mergeThreatDetectionEngineEnv(data, threatDetectionData.EngineConfig.Env) // Inherit APITarget from main engine config for GHE/custom endpoints. if threatDetectionData.EngineConfig.APITarget == "" && data.EngineConfig != nil { threatDetectionData.EngineConfig.APITarget = data.EngineConfig.APITarget diff --git a/pkg/workflow/threat_detection_helpers.go b/pkg/workflow/threat_detection_helpers.go index 3b4baefd55f..9a69d6eeed6 100644 --- a/pkg/workflow/threat_detection_helpers.go +++ b/pkg/workflow/threat_detection_helpers.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" + "maps" "strings" "github.com/github/gh-aw/pkg/constants" @@ -70,7 +71,14 @@ func isThreatDetectionExplicitlyDisabledInConfigs(configs []string) bool { } func getThreatDetectionAdditionalAllowedDomains(data *WorkflowData) []string { - if !engineEnvHasKey(data, constants.CopilotProviderBaseURL) || data == nil || data.NetworkPermissions == nil { + if data == nil || data.NetworkPermissions == nil { + return []string{} + } + + hasCustomTarget := extractAPITargetHost(data, "OPENAI_BASE_URL") != "" || + extractAPITargetHost(data, "ANTHROPIC_BASE_URL") != "" || + extractAPITargetHost(data, constants.CopilotProviderBaseURL) != "" + if !hasCustomTarget { return []string{} } @@ -101,6 +109,22 @@ func canReuseThreatDetectionEngineConfigForExternalDetector(data *WorkflowData, (data.SafeOutputs.ThreatDetection.EngineConfig.ID == "" || data.SafeOutputs.ThreatDetection.EngineConfig.ID == engineID) } +func mergeThreatDetectionEngineEnv(data *WorkflowData, detectionEnv map[string]string) map[string]string { + if data == nil || data.EngineConfig == nil || len(data.EngineConfig.Env) == 0 { + return detectionEnv + } + if len(detectionEnv) == 0 { + merged := make(map[string]string, len(data.EngineConfig.Env)) + maps.Copy(merged, data.EngineConfig.Env) + return merged + } + + merged := make(map[string]string, len(data.EngineConfig.Env)+len(detectionEnv)) + maps.Copy(merged, data.EngineConfig.Env) + maps.Copy(merged, detectionEnv) + return merged +} + // engineCoreSecretVarNames returns the secret-backed env var names for the given engine ID // that must be excluded from the AWF container via --exclude-env. These are the credentials // that AWF's API proxy intercepts, so the container itself does not need them. diff --git a/pkg/workflow/threat_detection_test.go b/pkg/workflow/threat_detection_test.go index d4a0410c671..df6fadd0395 100644 --- a/pkg/workflow/threat_detection_test.go +++ b/pkg/workflow/threat_detection_test.go @@ -2087,6 +2087,70 @@ func TestBuildExternalDetectorExecutionStepPropagatesRunnerTopology(t *testing.T }) } +func TestBuildExternalDetectorExecutionStepInheritsOpenAIBaseURLForAPITarget(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + AI: "codex", + EngineConfig: &EngineConfig{ + ID: "codex", + Env: map[string]string{ + "OPENAI_BASE_URL": "https://llm-router.internal.example.com/v1", + }, + }, + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{ + EngineConfig: &EngineConfig{ + ID: "codex", + Env: map[string]string{ + "CUSTOM_FLAG": "1", + }, + }, + }, + }, + } + + steps := compiler.buildExternalDetectorExecutionStep(data) + if len(steps) == 0 { + t.Fatal("expected non-empty steps") + } + allSteps := strings.Join(steps, "") + + if !strings.Contains(allSteps, "apiProxy") { + t.Fatalf("expected external detector AWF config to include api-proxy target; got:\n%s", allSteps) + } + if !strings.Contains(allSteps, "llm-router.internal.example.com") { + t.Fatalf("expected external detector AWF config to include host from OPENAI_BASE_URL; got:\n%s", allSteps) + } +} + +func TestGetThreatDetectionAdditionalAllowedDomains_WithOpenAIBaseURL(t *testing.T) { + data := &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "OPENAI_BASE_URL": "https://llm-router.internal.example.com/v1", + }, + }, + NetworkPermissions: &NetworkPermissions{ + Allowed: []string{ + "llm-router.internal.example.com", + "api.openai.com", + "${{ inputs.allowed_domains }}", + "chatgpt.com", + }, + }, + } + + got := getThreatDetectionAdditionalAllowedDomains(data) + want := []string{ + "llm-router.internal.example.com", + "api.openai.com", + "chatgpt.com", + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("expected additional allowed domains %v, got %v", want, got) + } +} + func TestAppendThreatDetectionRWMount(t *testing.T) { threatDetectionMount := constants.ThreatDetectionDir + ":" + constants.ThreatDetectionDir + ":rw" From 5d56a8c9474ea86731ab8fcca490c2e5a0df8050 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:15:43 +0000 Subject: [PATCH 3/5] Propagate detection env for custom API targets Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/threat_detection_external.go | 30 +------- pkg/workflow/threat_detection_helpers.go | 24 ++++++- pkg/workflow/threat_detection_test.go | 84 +++++++++++++++-------- 3 files changed, 80 insertions(+), 58 deletions(-) diff --git a/pkg/workflow/threat_detection_external.go b/pkg/workflow/threat_detection_external.go index 108ab0a38cf..cc231ba9d28 100644 --- a/pkg/workflow/threat_detection_external.go +++ b/pkg/workflow/threat_detection_external.go @@ -205,20 +205,7 @@ func (c *Compiler) buildInstallDetectionEngineForExternalDetectorStep(data *Work threatDetectionData.EngineConfig = &EngineConfig{ID: engineID} if canReuseThreatDetectionEngineConfigForExternalDetector(data, engineID) { - ec := data.SafeOutputs.ThreatDetection.EngineConfig - threatDetectionData.EngineConfig = &EngineConfig{ - ID: engineID, - Model: ec.Model, - Version: ec.Version, - Env: ec.Env, - Config: ec.Config, - Args: ec.Args, - Command: ec.Command, - APITarget: ec.APITarget, - HarnessScript: ec.HarnessScript, - Driver: ec.Driver, - CopilotSDK: ec.CopilotSDK, - } + threatDetectionData.EngineConfig = cloneThreatDetectionEngineConfig(engineID, data.SafeOutputs.ThreatDetection.EngineConfig) } threatDetectionData.EngineConfig.Env = mergeThreatDetectionEngineEnv(data, threatDetectionData.EngineConfig.Env) if threatDetectionData.EngineConfig.APITarget == "" && data.EngineConfig != nil { @@ -293,20 +280,7 @@ func (c *Compiler) buildExternalDetectorExecutionStep(data *WorkflowData) []stri // Inherit engine config overrides from threat-detection config when set. if canReuseThreatDetectionEngineConfigForExternalDetector(data, engineID) { - ec := data.SafeOutputs.ThreatDetection.EngineConfig - threatDetectionData.EngineConfig = &EngineConfig{ - ID: engineID, - Model: ec.Model, - Version: ec.Version, - Env: ec.Env, - Config: ec.Config, - Args: ec.Args, - Command: ec.Command, - APITarget: ec.APITarget, - HarnessScript: ec.HarnessScript, - Driver: ec.Driver, - CopilotSDK: ec.CopilotSDK, - } + threatDetectionData.EngineConfig = cloneThreatDetectionEngineConfig(engineID, data.SafeOutputs.ThreatDetection.EngineConfig) } threatDetectionData.EngineConfig.Env = mergeThreatDetectionEngineEnv(data, threatDetectionData.EngineConfig.Env) // Inherit APITarget from main engine config for GHE/custom endpoints. diff --git a/pkg/workflow/threat_detection_helpers.go b/pkg/workflow/threat_detection_helpers.go index 9a69d6eeed6..5eb0230c2cd 100644 --- a/pkg/workflow/threat_detection_helpers.go +++ b/pkg/workflow/threat_detection_helpers.go @@ -109,14 +109,20 @@ func canReuseThreatDetectionEngineConfigForExternalDetector(data *WorkflowData, (data.SafeOutputs.ThreatDetection.EngineConfig.ID == "" || data.SafeOutputs.ThreatDetection.EngineConfig.ID == engineID) } +// mergeThreatDetectionEngineEnv composes detection engine env vars from the main +// engine env and detection-specific overrides. +// +// Detection values take precedence when keys overlap. When detectionEnv is empty, +// it still returns a copy of the main env map to avoid aliasing/mutation of the +// parent WorkflowData.EngineConfig.Env by downstream detection-specific updates. func mergeThreatDetectionEngineEnv(data *WorkflowData, detectionEnv map[string]string) map[string]string { if data == nil || data.EngineConfig == nil || len(data.EngineConfig.Env) == 0 { return detectionEnv } if len(detectionEnv) == 0 { - merged := make(map[string]string, len(data.EngineConfig.Env)) - maps.Copy(merged, data.EngineConfig.Env) - return merged + // Return a copy (not the original map) so subsequent detection-specific + // env merges cannot mutate the main engine's env map by aliasing. + return maps.Clone(data.EngineConfig.Env) } merged := make(map[string]string, len(data.EngineConfig.Env)+len(detectionEnv)) @@ -125,6 +131,18 @@ func mergeThreatDetectionEngineEnv(data *WorkflowData, detectionEnv map[string]s return merged } +// cloneThreatDetectionEngineConfig returns a shallow copy of source with engine ID +// normalized to the provided detection engineID. If source is nil, it returns a +// minimal config containing only the ID. +func cloneThreatDetectionEngineConfig(engineID string, source *EngineConfig) *EngineConfig { + if source == nil { + return &EngineConfig{ID: engineID} + } + cloned := *source + cloned.ID = engineID + return &cloned +} + // engineCoreSecretVarNames returns the secret-backed env var names for the given engine ID // that must be excluded from the AWF container via --exclude-env. These are the credentials // that AWF's API proxy intercepts, so the container itself does not need them. diff --git a/pkg/workflow/threat_detection_test.go b/pkg/workflow/threat_detection_test.go index df6fadd0395..3d34606afec 100644 --- a/pkg/workflow/threat_detection_test.go +++ b/pkg/workflow/threat_detection_test.go @@ -2087,7 +2087,7 @@ func TestBuildExternalDetectorExecutionStepPropagatesRunnerTopology(t *testing.T }) } -func TestBuildExternalDetectorExecutionStepInheritsOpenAIBaseURLForAPITarget(t *testing.T) { +func TestExternalDetectorInheritsOpenAIBaseURL(t *testing.T) { compiler := NewCompiler() data := &WorkflowData{ AI: "codex", @@ -2113,41 +2113,71 @@ func TestBuildExternalDetectorExecutionStepInheritsOpenAIBaseURLForAPITarget(t * if len(steps) == 0 { t.Fatal("expected non-empty steps") } - allSteps := strings.Join(steps, "") + stepsContent := strings.Join(steps, "") - if !strings.Contains(allSteps, "apiProxy") { - t.Fatalf("expected external detector AWF config to include api-proxy target; got:\n%s", allSteps) + if !strings.Contains(stepsContent, "apiProxy") { + t.Fatalf("expected external detector AWF config to include api-proxy target; got:\n%s", stepsContent) } - if !strings.Contains(allSteps, "llm-router.internal.example.com") { - t.Fatalf("expected external detector AWF config to include host from OPENAI_BASE_URL; got:\n%s", allSteps) + if !strings.Contains(stepsContent, "llm-router.internal.example.com") { + t.Fatalf("expected external detector AWF config to include host from OPENAI_BASE_URL; got:\n%s", stepsContent) } } -func TestGetThreatDetectionAdditionalAllowedDomains_WithOpenAIBaseURL(t *testing.T) { - data := &WorkflowData{ - EngineConfig: &EngineConfig{ - Env: map[string]string{ - "OPENAI_BASE_URL": "https://llm-router.internal.example.com/v1", - }, +func TestGetThreatDetectionAdditionalAllowedDomains_WithCustomProviderBaseURL(t *testing.T) { + tests := []struct { + name string + baseURLVar string + baseURLValue string + }{ + { + name: "openai base URL", + baseURLVar: "OPENAI_BASE_URL", + baseURLValue: "https://llm-router.internal.example.com/v1", }, - NetworkPermissions: &NetworkPermissions{ - Allowed: []string{ - "llm-router.internal.example.com", - "api.openai.com", - "${{ inputs.allowed_domains }}", - "chatgpt.com", - }, + { + name: "anthropic base URL", + baseURLVar: "ANTHROPIC_BASE_URL", + baseURLValue: "https://anthropic-router.internal.example.com/v1", + }, + { + name: "copilot provider base URL", + baseURLVar: constants.CopilotProviderBaseURL, + baseURLValue: "https://copilot-router.internal.example.com/v1", }, } - got := getThreatDetectionAdditionalAllowedDomains(data) - want := []string{ - "llm-router.internal.example.com", - "api.openai.com", - "chatgpt.com", - } - if !reflect.DeepEqual(got, want) { - t.Fatalf("expected additional allowed domains %v, got %v", want, got) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := &WorkflowData{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + tt.baseURLVar: tt.baseURLValue, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Allowed: []string{ + "llm-router.internal.example.com", + "anthropic-router.internal.example.com", + "copilot-router.internal.example.com", + "api.openai.com", + "${{ inputs.allowed_domains }}", + "chatgpt.com", + }, + }, + } + + got := getThreatDetectionAdditionalAllowedDomains(data) + want := []string{ + "llm-router.internal.example.com", + "anthropic-router.internal.example.com", + "copilot-router.internal.example.com", + "api.openai.com", + "chatgpt.com", + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("expected additional allowed domains %v, got %v", want, got) + } + }) } } From 60216b77bc0b4397f38274803e65f33cd564c056 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:13:59 +0000 Subject: [PATCH 4/5] Initial plan for review feedback fixes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../workflows/daily-byok-ollama-test.lock.yml | 5 +- pkg/workflow/debug_test.go | 46 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 pkg/workflow/debug_test.go diff --git a/.github/workflows/daily-byok-ollama-test.lock.yml b/.github/workflows/daily-byok-ollama-test.lock.yml index c5aebfda113..8c79ce0f3de 100644 --- a/.github/workflows/daily-byok-ollama-test.lock.yml +++ b/.github/workflows/daily-byok-ollama-test.lock.yml @@ -1465,8 +1465,9 @@ jobs: AWF_REFLECT_ENABLED: 1 COPILOT_AGENT_RUNNER_TYPE: STANDALONE COPILOT_DUMMY_BYOK: dummy-byok-key-for-offline-mode - COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} - COPILOT_MODEL: ${{ vars.GH_AW_MODEL_DETECTION_COPILOT || vars.GH_AW_DEFAULT_MODEL_COPILOT || 'claude-sonnet-4.6' }} + COPILOT_MODEL: qwen2.5:0.5b + COPILOT_PROVIDER_API_KEY: ${{ env.OLLAMA_API_KEY }} + COPILOT_PROVIDER_BASE_URL: http://host.docker.internal:11434/v1 GH_AW_LLM_PROVIDER: github GH_AW_MAX_AI_CREDITS: ${{ vars.GH_AW_DEFAULT_DETECTION_MAX_AI_CREDITS || '400' }} GH_AW_MAX_TURNS: ${{ vars.GH_AW_DEFAULT_MAX_TURNS || '' }} diff --git a/pkg/workflow/debug_test.go b/pkg/workflow/debug_test.go new file mode 100644 index 00000000000..58ff0b3813f --- /dev/null +++ b/pkg/workflow/debug_test.go @@ -0,0 +1,46 @@ +package workflow + +import ( + "fmt" + "strings" + "testing" +) + +func TestExternalDetectorInheritsOpenAIBaseURL_Debug(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + AI: "codex", + EngineConfig: &EngineConfig{ + ID: "codex", + Env: map[string]string{ + "OPENAI_BASE_URL": "https://llm-router.internal.example.com/v1", + }, + }, + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{ + EngineConfig: &EngineConfig{ + ID: "codex", + Env: map[string]string{ + "CUSTOM_FLAG": "1", + }, + }, + }, + }, + } + + steps := compiler.buildExternalDetectorExecutionStep(data) + stepsContent := strings.Join(steps, "") + // Print the relevant fragment around apiProxy + idx := strings.Index(stepsContent, "apiProxy") + if idx != -1 { + start := idx - 20 + if start < 0 { + start = 0 + } + end := idx + 200 + if end > len(stepsContent) { + end = len(stepsContent) + } + fmt.Printf("FRAGMENT: %s\n", stepsContent[start:end]) + } +} From cb5642e8b9597256ab9a8e6e527b29cb96657752 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:35:33 +0000 Subject: [PATCH 5/5] Address review feedback: merged env for detection allowlist/needs, strengthen test assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_jobs_test.go | 25 +++--- pkg/workflow/debug_test.go | 46 ------------ pkg/workflow/threat_detection_helpers.go | 15 +++- pkg/workflow/threat_detection_job.go | 14 ++-- pkg/workflow/threat_detection_test.go | 96 ++++++++++++++++++++++-- 5 files changed, 126 insertions(+), 70 deletions(-) delete mode 100644 pkg/workflow/debug_test.go diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index c7e527f401a..24540f7585b 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -4176,8 +4176,9 @@ func TestBuildDetectionJobEngineEnvBuiltinWarning(t *testing.T) { } // TestBuildDetectionJobEngineEnvNeedsExpression verifies that the detection job scans the -// effective detection engine env, so safe-outputs.threat-detection.engine overrides the -// top-level engine env when resolving custom job dependencies. +// effective merged detection engine env (main + detection-specific overrides) when resolving +// custom job dependencies. Both the detection-specific env and the main engine env are +// included so that expressions merged into the detection step have their jobs listed in needs. func TestBuildDetectionJobEngineEnvNeedsExpression(t *testing.T) { compiler := NewCompiler() compiler.stepOrderTracker = NewStepOrderTracker() @@ -4233,11 +4234,13 @@ func TestBuildDetectionJobEngineEnvNeedsExpression(t *testing.T) { require.NoError(t, err, "buildDetectionJob should succeed") require.NotNil(t, job, "detection job should be created") - // The detection job must use the threat-detection override env, not the top-level env. + // The detection job must scan the merged env (main + detection-specific overrides). + // Both select_model (from detection env) and ignored_job (from main env, merged in) + // must appear in needs so their expressions resolve correctly in the detection step. assert.Contains(t, job.Needs, "select_model", "detection job must directly depend on select_model referenced in threat-detection.engine.env") - assert.NotContains(t, job.Needs, "ignored_job", - "detection job must ignore top-level engine.env references when threat-detection.engine overrides env") + assert.Contains(t, job.Needs, "ignored_job", + "detection job must also depend on ignored_job merged from main engine.env") assert.Contains(t, job.Needs, string(constants.AgentJobName), "detection job must still depend on agent") assert.Contains(t, job.Needs, string(constants.ActivationJobName), @@ -4292,6 +4295,8 @@ func TestBuildDetectionJobEngineEnvNeedsNotDuplicated(t *testing.T) { // TestBuildDetectionJobEngineEnvNeedsIntegration is an end-to-end integration test that // compiles a workflow where engine.env references a custom job output, and verifies that the // compiled lock file includes the custom job as a direct dependency of the detection job. +// With merged-env dependency scanning, both the detection-specific env and main engine env +// references appear in detection needs. func TestBuildDetectionJobEngineEnvNeedsIntegration(t *testing.T) { tmpDir := testutil.TempDir(t, "detection_engine_env_needs_test") @@ -4306,16 +4311,16 @@ permissions: engine: id: copilot env: - IGNORED_MODEL: ${{ needs.ignored_job.outputs.model }} + MAIN_MODEL: ${{ needs.main_job.outputs.model }} strict: false jobs: - ignored_job: + main_job: runs-on: ubuntu-latest outputs: model: ${{ steps.pick.outputs.model }} steps: - id: pick - run: echo "model=ignored" >> "$GITHUB_OUTPUT" + run: echo "model=from-main" >> "$GITHUB_OUTPUT" select_model: runs-on: ubuntu-latest needs: pre_activation @@ -4369,6 +4374,6 @@ This workflow tests that engine.env needs expressions create detection job depen assert.Contains(t, detectionNeeds, "select_model", "detection job must list select_model in needs when referenced via threat-detection.engine.env") - assert.NotContains(t, detectionNeeds, "ignored_job", - "detection job must not inherit top-level engine.env dependencies when threat-detection.engine overrides env") + assert.Contains(t, detectionNeeds, "main_job", + "detection job must also list main_job in needs when main engine.env is merged into detection env") } diff --git a/pkg/workflow/debug_test.go b/pkg/workflow/debug_test.go deleted file mode 100644 index 58ff0b3813f..00000000000 --- a/pkg/workflow/debug_test.go +++ /dev/null @@ -1,46 +0,0 @@ -package workflow - -import ( - "fmt" - "strings" - "testing" -) - -func TestExternalDetectorInheritsOpenAIBaseURL_Debug(t *testing.T) { - compiler := NewCompiler() - data := &WorkflowData{ - AI: "codex", - EngineConfig: &EngineConfig{ - ID: "codex", - Env: map[string]string{ - "OPENAI_BASE_URL": "https://llm-router.internal.example.com/v1", - }, - }, - SafeOutputs: &SafeOutputsConfig{ - ThreatDetection: &ThreatDetectionConfig{ - EngineConfig: &EngineConfig{ - ID: "codex", - Env: map[string]string{ - "CUSTOM_FLAG": "1", - }, - }, - }, - }, - } - - steps := compiler.buildExternalDetectorExecutionStep(data) - stepsContent := strings.Join(steps, "") - // Print the relevant fragment around apiProxy - idx := strings.Index(stepsContent, "apiProxy") - if idx != -1 { - start := idx - 20 - if start < 0 { - start = 0 - } - end := idx + 200 - if end > len(stepsContent) { - end = len(stepsContent) - } - fmt.Printf("FRAGMENT: %s\n", stepsContent[start:end]) - } -} diff --git a/pkg/workflow/threat_detection_helpers.go b/pkg/workflow/threat_detection_helpers.go index 5eb0230c2cd..4e94ba29225 100644 --- a/pkg/workflow/threat_detection_helpers.go +++ b/pkg/workflow/threat_detection_helpers.go @@ -75,9 +75,18 @@ func getThreatDetectionAdditionalAllowedDomains(data *WorkflowData) []string { return []string{} } - hasCustomTarget := extractAPITargetHost(data, "OPENAI_BASE_URL") != "" || - extractAPITargetHost(data, "ANTHROPIC_BASE_URL") != "" || - extractAPITargetHost(data, constants.CopilotProviderBaseURL) != "" + // Evaluate the effective merged detection environment (main + detection-specific + // overrides) so that a custom base URL configured only in + // safe-outputs.threat-detection.engine.env also triggers domain propagation. + var detectionSpecificEnv map[string]string + if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.EngineConfig != nil { + detectionSpecificEnv = data.SafeOutputs.ThreatDetection.EngineConfig.Env + } + effectiveEnv := mergeThreatDetectionEngineEnv(data, detectionSpecificEnv) + + hasCustomTarget := effectiveEnv["OPENAI_BASE_URL"] != "" || + effectiveEnv["ANTHROPIC_BASE_URL"] != "" || + effectiveEnv[constants.CopilotProviderBaseURL] != "" if !hasCustomTarget { return []string{} } diff --git a/pkg/workflow/threat_detection_job.go b/pkg/workflow/threat_detection_job.go index f01ad31333a..b3063e2da49 100644 --- a/pkg/workflow/threat_detection_job.go +++ b/pkg/workflow/threat_detection_job.go @@ -75,15 +75,17 @@ func (c *Compiler) buildDetectionJob(data *WorkflowData) (*Job, error) { // Scan the effective detection engine env values for needs..outputs.* // expressions and add the referenced custom jobs as direct dependencies of the - // detection job. safe-outputs.threat-detection.engine overrides the top-level - // engine config for detection execution, so its env map must win here as well. - detectionEngineConfig := data.EngineConfig + // detection job. Use the merged env (main + detection-specific overrides) so + // that expressions from the main engine.env are not missed when a detection + // engine config exists. + var detectionSpecificEnv map[string]string if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.EngineConfig != nil { - detectionEngineConfig = data.SafeOutputs.ThreatDetection.EngineConfig + detectionSpecificEnv = data.SafeOutputs.ThreatDetection.EngineConfig.Env } - if detectionEngineConfig != nil && len(detectionEngineConfig.Env) > 0 { + effectiveDetectionEnv := mergeThreatDetectionEngineEnv(data, detectionSpecificEnv) + if len(effectiveDetectionEnv) > 0 { var engineEnvBuilder strings.Builder - for _, envValue := range detectionEngineConfig.Env { + for _, envValue := range effectiveDetectionEnv { engineEnvBuilder.WriteByte('\n') engineEnvBuilder.WriteString(envValue) } diff --git a/pkg/workflow/threat_detection_test.go b/pkg/workflow/threat_detection_test.go index 3d34606afec..15167611347 100644 --- a/pkg/workflow/threat_detection_test.go +++ b/pkg/workflow/threat_detection_test.go @@ -4,6 +4,7 @@ package workflow import ( "reflect" + "slices" "strings" "testing" @@ -2115,11 +2116,12 @@ func TestExternalDetectorInheritsOpenAIBaseURL(t *testing.T) { } stepsContent := strings.Join(steps, "") - if !strings.Contains(stepsContent, "apiProxy") { - t.Fatalf("expected external detector AWF config to include api-proxy target; got:\n%s", stepsContent) - } - if !strings.Contains(stepsContent, "llm-router.internal.example.com") { - t.Fatalf("expected external detector AWF config to include host from OPENAI_BASE_URL; got:\n%s", stepsContent) + // Assert the specific serialized apiProxy.targets.openai.host entry to verify + // that OPENAI_BASE_URL is reflected as a custom target in the AWF config, not + // just that the hostname appears somewhere in the step output. + wantTarget := `\"targets\":{\"openai\":{\"host\":\"llm-router.internal.example.com\"` + if !strings.Contains(stepsContent, wantTarget) { + t.Fatalf("expected external detector AWF config to include apiProxy.targets.openai.host=%q; got:\n%s", "llm-router.internal.example.com", stepsContent) } } @@ -2181,6 +2183,90 @@ func TestGetThreatDetectionAdditionalAllowedDomains_WithCustomProviderBaseURL(t } } +// TestGetThreatDetectionAdditionalAllowedDomains_DetectionOnlyBaseURL verifies that +// a custom base URL configured only in safe-outputs.threat-detection.engine.env (not +// in the main engine env) still triggers domain propagation. This is the case where +// the effective merged detection env must be evaluated, not just data.EngineConfig.Env. +func TestGetThreatDetectionAdditionalAllowedDomains_DetectionOnlyBaseURL(t *testing.T) { + data := &WorkflowData{ + // Main engine has no custom base URL. + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "SOME_OTHER_VAR": "value", + }, + }, + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{ + EngineConfig: &EngineConfig{ + Env: map[string]string{ + "OPENAI_BASE_URL": "https://detection-router.internal.example.com/v1", + }, + }, + }, + }, + NetworkPermissions: &NetworkPermissions{ + Allowed: []string{ + "detection-router.internal.example.com", + "api.openai.com", + }, + }, + } + + got := getThreatDetectionAdditionalAllowedDomains(data) + want := []string{ + "detection-router.internal.example.com", + "api.openai.com", + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("expected additional allowed domains %v, got %v (detection-only base URL must also trigger propagation)", want, got) + } +} + +// TestBuildDetectionJobNeedsIncludesMainEngineEnvJobs verifies that when the main +// engine env contains a needs expression and a detection-specific engine config also +// exists, the referenced custom job is still added to the detection job's needs. +// This tests the merged-env dependency scan path. +func TestBuildDetectionJobNeedsIncludesMainEngineEnvJobs(t *testing.T) { + compiler := NewCompiler() + data := &WorkflowData{ + AI: "codex", + EngineConfig: &EngineConfig{ + ID: "codex", + Env: map[string]string{ + // This expression references a custom job "router" from the main engine env. + "OPENAI_BASE_URL": "${{ needs.router.outputs.url }}", + }, + }, + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{ + // Detection-specific config exists, which previously caused the scan + // to use only this env, missing the main engine env expression above. + EngineConfig: &EngineConfig{ + ID: "codex", + Env: map[string]string{ + "CUSTOM_FLAG": "1", + }, + }, + }, + }, + Jobs: map[string]any{ + "router": map[string]any{}, + }, + } + + job, err := compiler.buildDetectionJob(data) + if err != nil { + t.Fatalf("buildDetectionJob() error: %v", err) + } + if job == nil { + t.Fatal("buildDetectionJob() returned nil job") + } + + if !slices.Contains(job.Needs, "router") { + t.Fatalf("expected detection job needs to include 'router' (referenced via main engine OPENAI_BASE_URL); got needs: %v", job.Needs) + } +} + func TestAppendThreatDetectionRWMount(t *testing.T) { threatDetectionMount := constants.ThreatDetectionDir + ":" + constants.ThreatDetectionDir + ":rw"