Fix external threat detection to honor custom engine base URLs and allowlist propagation

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")
 }

pkg/workflow/threat_detection_external.go

@@ -205,21 +205,9 @@ 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 {
 		threatDetectionData.EngineConfig.APITarget = data.EngineConfig.APITarget
 	}
@@ -292,12 +280,9 @@ 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,
-			APITarget: ec.APITarget,
-		}
+		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.
 	if threatDetectionData.EngineConfig.APITarget == "" && data.EngineConfig != nil {
 		threatDetectionData.EngineConfig.APITarget = data.EngineConfig.APITarget

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,23 @@ 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{}
+	}
+
+	// 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{}
 	}
 
@@ -101,6 +118,40 @@ 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 {
+		// 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))
+	maps.Copy(merged, data.EngineConfig.Env)
+	maps.Copy(merged, detectionEnv)
+	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.

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.<customJob>.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)
 		}