Isolate threat detection provider settings across engine identities

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan · Oct 7, 2026 · Oct 7, 2026 · Oct 7, 2026 · Oct 7, 2026 · Oct 7, 2026
commit 8c9f4a72721cd5b6092e2c758b0eeeb0ee2ea786
diff --git a/docs/src/content/docs/reference/threat-detection.md b/docs/src/content/docs/reference/threat-detection.md
@@ -244,6 +244,12 @@ The custom prompt is appended to the default threat detection instructions, prov
 
 Override the main workflow engine for threat detection:
 
+When the resolved detection engine matches the main engine, detection inherits `engine.env`, `api-target`, and model aliases. Detection-specific environment values take precedence, including empty values.
+
+When the engines differ, detection does not inherit the main engine's environment, API target, or custom model aliases. This prevents provider URLs, API keys, and custom headers from reaching an unrelated engine. Configure any required provider settings under `safe-outputs.threat-detection.engine.env`; built-in model aliases remain available.
+
+The conclusion job emits a warning when detection runs but produces no security verdict, including engine failures and unparseable results. Intentionally skipped detection does not emit this warning. The warning does not change `continue-on-error` behavior.
+
 **String Format:**
 
 ```yaml wrap

diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go
@@ -409,7 +409,7 @@ func (c *Compiler) shouldEmitPiThreatDetectionAuthWarning(workflowData *Workflow
 		return false
 	}
 
-	effectiveEnv := mergeThreatDetectionEngineEnv(workflowData, detectionEnv)
+	effectiveEnv := mergeThreatDetectionEngineEnv(workflowData, c.getThreatDetectionEngineID(workflowData), detectionEnv)
 	if strings.TrimSpace(effectiveEnv[constants.CopilotGitHubToken]) != "" {
 		return false
 	}

diff --git a/pkg/workflow/compiler_validators_test.go b/pkg/workflow/compiler_validators_test.go
@@ -743,7 +743,7 @@ func TestEmitGeneralToolWarnings_PiThreatDetectionAuthWarning(t *testing.T) {
 			},
 		},
 		{
-			name: "does not warn with explicit Copilot token",
+			name: "warns when Copilot token is configured only on the main Pi engine",
 			data: &WorkflowData{
 				AI: "pi",
 				EngineConfig: &EngineConfig{
@@ -757,6 +757,25 @@ func TestEmitGeneralToolWarnings_PiThreatDetectionAuthWarning(t *testing.T) {
 					ThreatDetection: &ThreatDetectionConfig{},
 				},
 			},
+			wantWarning: true,
+		},
+		{
+			name: "does not warn with explicit detection Copilot token",
+			data: &WorkflowData{
+				AI:           "pi",
+				EngineConfig: &EngineConfig{ID: "pi"},
+				Permissions:  "permissions:\n  contents: read\n",
+				SafeOutputs: &SafeOutputsConfig{
+					ThreatDetection: &ThreatDetectionConfig{
+						EngineConfig: &EngineConfig{
+							ID: "copilot",
+							Env: map[string]string{
+								"COPILOT_GITHUB_TOKEN": "${{ secrets.DETECTION_COPILOT_TOKEN }}",
+							},
+						},
+					},
+				},
+			},
 		},
 		{
 			name: "does not warn with Copilot BYOK credentials",

diff --git a/pkg/workflow/evals_steps.go b/pkg/workflow/evals_steps.go
@@ -219,7 +219,7 @@ func (c *Compiler) buildEvalsEngineSteps(data *WorkflowData) []string { //nolint
 		ModelCosts:        data.ModelCosts,      // propagate pricing providers so evals awf-config.json can resolve AI-credit costs
 		CompiledVersion:   data.CompiledVersion, // propagate compiler version so install steps can inject GH_AW_COMPILED_VERSION
 		NetworkPermissions: &NetworkPermissions{
-			Allowed: getThreatDetectionAdditionalAllowedDomains(data),
+			Allowed: getThreatDetectionAdditionalAllowedDomains(data, engineID),
 		},
 		SandboxConfig: &SandboxConfig{
 			Agent: &AgentSandboxConfig{

diff --git a/pkg/workflow/notify_comment_conclusion_helpers.go b/pkg/workflow/notify_comment_conclusion_helpers.go
@@ -56,6 +56,11 @@ func (c *Compiler) buildConclusionSetupSteps(data *WorkflowData) []string {
 	// expects, letting detection-phase usage surface in the usage artifact and count toward
 	// the AI-credits budget cap (see gh-aw#54047).
 	if IsDetectionJobEnabled(data.SafeOutputs) {
+		steps = append(steps,
+			"      - name: Warn if threat detection produced no verdict\n",
+			"        if: always() && needs.detection.result != 'skipped' && (needs.detection.outputs.detection_reason == 'agent_failure' || needs.detection.outputs.detection_reason == 'parse_error' || needs.detection.outputs.detection_conclusion == '')\n",
+			"        run: echo \"::warning::Threat detection produced no security verdict. Review the detection job logs before trusting the agent outputs.\"\n",
+		)
 		steps = append(steps, buildDetectionArtifactDownloadSteps(artifactPrefixExprForDownstreamJob(data), c.getActionPin)...)
 	}
 	steps = append(steps, buildUsageArtifactInputDownloadSteps(artifactPrefixExprForDownstreamJob(data), data.Evals != nil && data.Evals.HasEvals(), usageExperimentArtifactName(data), c.getActionPin)...)

diff --git a/pkg/workflow/threat_detection_engine_env_test.go b/pkg/workflow/threat_detection_engine_env_test.go
@@ -10,6 +10,33 @@ import (
 	yamlv3 "gopkg.in/yaml.v3"
 )
 
+func TestConclusionWarnsWhenDetectionHasNoVerdict(t *testing.T) {
+	compiler := NewCompiler()
+	data := &WorkflowData{
+		SafeOutputs: &SafeOutputsConfig{
+			ThreatDetection: &ThreatDetectionConfig{},
+		},
+	}
+	steps := strings.Join(compiler.buildConclusionSetupSteps(data), "")
+	for _, want := range []string{
+		"Warn if threat detection produced no verdict",
+		"if: always() && needs.detection.result != 'skipped'",
+		"needs.detection.outputs.detection_reason == 'agent_failure'",
+		"needs.detection.outputs.detection_reason == 'parse_error'",
+		"needs.detection.outputs.detection_conclusion == ''",
+		"::warning::Threat detection produced no security verdict.",
+	} {
+		if !strings.Contains(steps, want) {
+			t.Errorf("expected conclusion steps to contain %q", want)
+		}
+	}
+	data.SafeOutputs.ThreatDetection = nil
+	steps = strings.Join(compiler.buildConclusionSetupSteps(data), "")
+	if strings.Contains(steps, "Warn if threat detection produced no verdict") {
+		t.Error("disabled detection must not produce a missing-verdict warning")
+	}
+}
+
 // TestBuildDetectionEngineExecutionStepPropagatesAPITarget verifies that when engine.api-target
 // is configured on the main engine, the threat detection AWF invocation also receives
 // --copilot-api-target and the GHE domains in --allow-domains.

diff --git a/pkg/workflow/threat_detection_external.go b/pkg/workflow/threat_detection_external.go
@@ -136,6 +136,11 @@ func buildThreatDetectionWorkflowData(data *WorkflowData, engineID string) *Work
 			},
 		},
 	}
+	if ResolveEngineID(data) == engineID {
+		detectionData.ModelMappings = data.ModelMappings
+	} else {
+		detectionData.ModelMappings = MergeImportedModelAliases(nil, nil)
+	}
 	if pullPolicy := dockerImagePullPolicy(data); pullPolicy != "" {
 		detectionData.RawFrontmatter = map[string]any{"docker-image-pull-policy": pullPolicy}
 	}
@@ -488,16 +493,12 @@ func (c *Compiler) buildExternalDetectorExecutionStep(data *WorkflowData) []stri
 		resolvedDetectionModel = extractPiModelID(resolvedDetectionModel)
 	}
 	threatDetectionData.Model = resolvedDetectionModel
-	// Propagate the model alias map so the detection AWF config includes
-	// apiProxy.models, enabling the harness to resolve aliases (e.g. "haiku") to
-	// concrete model IDs before the Copilot CLI makes inference requests.
-	threatDetectionData.ModelMappings = data.ModelMappings
 	// Propagate default AI credits pricing so the detection AWF config includes
 	// apiProxy.defaultAiCreditsPricing when the main workflow configures it.
 	threatDetectionData.DefaultAiCreditsPricing = data.DefaultAiCreditsPricing
 
 	threatDetectionData.NetworkPermissions = &NetworkPermissions{
-		Allowed: getThreatDetectionAdditionalAllowedDomains(data),
+		Allowed: getThreatDetectionAdditionalAllowedDomains(data, engineID),
 	}
 	// Add a read-write mount so the threat-detect binary can write
 	// detection_result.json inside the container and it becomes visible

diff --git a/pkg/workflow/threat_detection_external_detector_config_test.go b/pkg/workflow/threat_detection_external_detector_config_test.go
@@ -3,12 +3,134 @@
 package workflow
 
 import (
+	"maps"
 	"strings"
 	"testing"
 
 	"github.com/github/gh-aw/pkg/workflow/compilerenv"
 )
 
+func TestDetectionProviderSettingsIsolatedAcrossEngines(t *testing.T) {
+	compiler := NewCompiler()
+	for _, mainEngine := range []string{"claude", "codex", "copilot", "pi"} {
+		for _, detectionEngine := range []string{"claude", "codex", "copilot"} {
+			if mainEngine == detectionEngine {
+				continue
+			}
+			t.Run(mainEngine+" to "+detectionEngine, func(t *testing.T) {
+				data := &WorkflowData{
+					AI: mainEngine,
+					EngineConfig: &EngineConfig{
+						ID:        mainEngine,
+						APITarget: "main-provider.example.com",
+						Env: map[string]string{
+							"ANTHROPIC_BASE_URL":       "https://main-provider.example.com/api",
+							"OPENAI_BASE_URL":          "https://main-provider.example.com/v1",
+							"ANTHROPIC_API_KEY":        "${{ secrets.MAIN_PROVIDER_KEY }}",
+							"OPENAI_API_KEY":           "${{ secrets.MAIN_PROVIDER_KEY }}",
+							"ANTHROPIC_CUSTOM_HEADERS": "X-Provider: main-only",
+						},
+					},
+					ModelMappings: map[string][]string{
+						"sonnet": {"main-only-model"},
+					},
+					SafeOutputs: &SafeOutputsConfig{
+						ThreatDetection: &ThreatDetectionConfig{
+							EngineConfig: &EngineConfig{
+								ID: detectionEngine,
+								Env: map[string]string{
+									"DETECTION_FLAG": "detection-only",
+								},
+							},
+						},
+					},
+					NetworkPermissions: &NetworkPermissions{
+						Allowed: []string{"main-provider.example.com"},
+					},
+				}
+				originalEnv := maps.Clone(data.EngineConfig.Env)
+				detectionData := buildExternalDetectorWorkflowData(data, detectionEngine)
+				if len(detectionData.EngineConfig.Env) != 1 || detectionData.EngineConfig.Env["DETECTION_FLAG"] != "detection-only" {
+					t.Fatalf("expected only detection-specific env, got %v", detectionData.EngineConfig.Env)
+				}
+				if detectionData.EngineConfig.APITarget != "" {
+					t.Fatal("main API target must not cross engine identities")
+				}
+				if len(getThreatDetectionAdditionalAllowedDomains(data, detectionEngine)) != 0 {
+					t.Fatal("main provider must not expand detection allowed domains")
+				}
+				if strings.Contains(strings.Join(detectionData.ModelMappings["sonnet"], ","), "main-only-model") {
+					t.Fatal("main model alias must not cross engine identities")
+				}
+				if len(detectionData.ModelMappings["sonnet"]) == 0 {
+					t.Fatal("built-in detection aliases must remain available")
+				}
+				for name, build := range map[string]func(*WorkflowData) []string{
+					"external": compiler.buildExternalDetectorExecutionStep,
+					"inline":   compiler.buildDetectionEngineExecutionStep,
+				} {
+					t.Run(name, func(t *testing.T) {
+						steps := strings.Join(build(data), "")
+						for _, forbidden := range []string{"main-provider.example.com", "MAIN_PROVIDER_KEY", "main-only", "main-only-model"} {
+							if strings.Contains(steps, forbidden) {
+								t.Errorf("main provider setting %q leaked into detection", forbidden)
+							}
+						}
+						if !strings.Contains(steps, "DETECTION_FLAG: detection-only") {
+							t.Error("detection-specific env was lost")
+						}
+					})
+				}
+				if !maps.Equal(originalEnv, data.EngineConfig.Env) {
+					t.Fatal("main engine env was mutated")
+				}
+				override := data.SafeOutputs.ThreatDetection.EngineConfig
+				override.Env["OPENAI_BASE_URL"] = "https://detection-provider.example.com/v1"
+				override.APITarget = "detection-target.example.com"
+				detectionData = buildExternalDetectorWorkflowData(data, detectionEngine)
+				if detectionData.EngineConfig.Env["OPENAI_BASE_URL"] != override.Env["OPENAI_BASE_URL"] ||
+					detectionData.EngineConfig.APITarget != override.APITarget {
+					t.Fatal("explicit detection provider overrides must be preserved across engines")
+				}
+			})
+		}
+	}
+}
+
+func TestMergeThreatDetectionEngineEnvSameEngine(t *testing.T) {
+	data := &WorkflowData{
+		AI: "claude",
+		EngineConfig: &EngineConfig{
+			ID: "claude",
+			Env: map[string]string{
+				"ANTHROPIC_BASE_URL": "https://main-provider.example.com",
+				"ANTHROPIC_API_KEY":  "${{ secrets.MAIN_PROVIDER_KEY }}",
+			},
+		},
+	}
+	for _, overrides := range []map[string]string{
+		nil,
+		{"ANTHROPIC_BASE_URL": ""},
+		{"ANTHROPIC_BASE_URL": "https://detection-provider.example.com"},
+	} {
+		merged := mergeThreatDetectionEngineEnv(data, "claude", overrides)
+		if merged["ANTHROPIC_API_KEY"] != data.EngineConfig.Env["ANTHROPIC_API_KEY"] {
+			t.Fatal("same-engine credentials must be inherited")
+		}
+		want := data.EngineConfig.Env["ANTHROPIC_BASE_URL"]
+		if override, ok := overrides["ANTHROPIC_BASE_URL"]; ok {
+			want = override
+		}
+		if merged["ANTHROPIC_BASE_URL"] != want {
+			t.Fatal("detection override must take precedence, including empty values")
+		}
+		merged["ANTHROPIC_BASE_URL"] = "mutated"
+		if data.EngineConfig.Env["ANTHROPIC_BASE_URL"] == "mutated" {
+			t.Fatal("merged env must not alias main engine env")
+		}
+	}
+}
+
 func TestExternalDetectorInheritsOpenAIBaseURL(t *testing.T) {
 	compiler := NewCompiler()
 	data := &WorkflowData{

diff --git a/pkg/workflow/threat_detection_firewall_test.go b/pkg/workflow/threat_detection_firewall_test.go
@@ -176,7 +176,7 @@ func TestGetThreatDetectionAdditionalAllowedDomains_WithCustomProviderBaseURL(t
 				},
 			}
 
-			got := getThreatDetectionAdditionalAllowedDomains(data)
+			got := getThreatDetectionAdditionalAllowedDomains(data, ResolveEngineID(data))
 			want := []string{
 				"llm-router.internal.example.com",
 				"anthropic-router.internal.example.com",
@@ -220,7 +220,7 @@ func TestGetThreatDetectionAdditionalAllowedDomains_DetectionOnlyBaseURL(t *test
 		},
 	}
 
-	got := getThreatDetectionAdditionalAllowedDomains(data)
+	got := getThreatDetectionAdditionalAllowedDomains(data, ResolveEngineID(data))
 	want := []string{
 		"detection-router.internal.example.com",
 		"api.openai.com",

diff --git a/pkg/workflow/threat_detection_helpers.go b/pkg/workflow/threat_detection_helpers.go
@@ -70,19 +70,18 @@ func isThreatDetectionExplicitlyDisabledInConfigs(configs []string) bool {
 	return false
 }
 
-func getThreatDetectionAdditionalAllowedDomains(data *WorkflowData) []string {
+func getThreatDetectionAdditionalAllowedDomains(data *WorkflowData, engineID string) []string {
 	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
+	// Evaluate the effective detection environment 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)
+	effectiveEnv := mergeThreatDetectionEngineEnv(data, engineID, detectionSpecificEnv)
 
 	hasCustomTarget := effectiveEnv["OPENAI_BASE_URL"] != "" ||
 		effectiveEnv["ANTHROPIC_BASE_URL"] != "" ||
@@ -112,13 +111,13 @@ func getThreatDetectionAdditionalAllowedDomains(data *WorkflowData) []string {
 }
 
 // mergeThreatDetectionEngineEnv composes detection engine env vars from the main
-// engine env and detection-specific overrides.
+// engine env and detection-specific overrides, inheriting only for the same engine.
 //
-// Detection values take precedence when keys overlap. When detectionEnv is empty,
+// Detection values take precedence when keys overlap. For the same engine, 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 {
+func mergeThreatDetectionEngineEnv(data *WorkflowData, engineID string, detectionEnv map[string]string) map[string]string {
+	if data == nil || ResolveEngineID(data) != engineID || data.EngineConfig == nil || len(data.EngineConfig.Env) == 0 {
 		return detectionEnv
 	}
 	if len(detectionEnv) == 0 {
@@ -155,8 +154,8 @@ func buildExternalDetectorWorkflowData(data *WorkflowData, engineID string) *Wor
 	if engineID == "codex" && NewCodexEngine().ResolveLLMProvider(d) != LLMProviderGitHub {
 		d.EngineConfig.LLMProvider = LLMProviderOpenAI
 	}
-	d.EngineConfig.Env = mergeThreatDetectionEngineEnv(data, d.EngineConfig.Env)
-	if d.EngineConfig.APITarget == "" && data.EngineConfig != nil {
+	d.EngineConfig.Env = mergeThreatDetectionEngineEnv(data, engineID, d.EngineConfig.Env)
+	if d.EngineConfig.APITarget == "" && data.EngineConfig != nil && ResolveEngineID(data) == engineID {
 		d.EngineConfig.APITarget = data.EngineConfig.APITarget
 	}
 	if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.MaxAICredits != 0 {

diff --git a/pkg/workflow/threat_detection_inline_engine.go b/pkg/workflow/threat_detection_inline_engine.go
@@ -56,7 +56,7 @@ func (c *Compiler) buildDetectionEngineExecutionStep(data *WorkflowData) []strin
 	// Detection resolves its own budgets independently of the main agent.
 	detectionEngineConfig := resolveExternalDetectorEngineConfig(data, engineSetting)
 	resetDetectionEngineTaskSettings(detectionEngineConfig)
-	detectionEngineConfig.Env = mergeThreatDetectionEngineEnv(data, detectionEngineConfig.Env)
+	detectionEngineConfig.Env = mergeThreatDetectionEngineEnv(data, engineSetting, detectionEngineConfig.Env)
 	if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.MaxAICredits != 0 {
 		detectionEngineConfig.MaxAICredits = data.SafeOutputs.ThreatDetection.MaxAICredits
 	}
@@ -95,10 +95,10 @@ func (c *Compiler) buildDetectionEngineExecutionStep(data *WorkflowData) []strin
 		resolvedDetectionModel = "detection"
 	}
 
-	// Inherit APITarget from the main engine config for GHE/custom endpoints if not already set.
+	// Inherit APITarget for GHE/custom endpoints only when the engines match.
 	// This ensures the threat detection AWF invocation receives the same --copilot-api-target
 	// and GHE-specific domains in --allow-domains as the main agent AWF invocation.
-	if detectionEngineConfig.APITarget == "" && data.EngineConfig != nil && data.EngineConfig.APITarget != "" {
+	if detectionEngineConfig.APITarget == "" && data.EngineConfig != nil && ResolveEngineID(data) == engineSetting {
 		detectionEngineConfig.APITarget = data.EngineConfig.APITarget
 	}
 	if engineSetting == "copilot" && originalEngineID == "pi" {
@@ -120,7 +120,7 @@ func (c *Compiler) buildDetectionEngineExecutionStep(data *WorkflowData) []strin
 	// bash: ["*"] allows all shell commands — AWF's network firewall is the primary
 	// constraint, so restricting individual bash commands inside the sandbox adds friction
 	// without meaningful security benefit.
-	// ModelMappings is propagated so the detection awf-config.json includes the alias map
+	// For matching engines, ModelMappings is propagated so the detection awf-config.json includes the alias map
 	// (apiProxy.models). Without it, copilot_harness.cjs cannot resolve alias model names
 	// (e.g. "small") to concrete ids before spawning the Copilot CLI in the detection job.
 	threatDetectionData := buildThreatDetectionWorkflowData(data, engineSetting)
@@ -129,13 +129,12 @@ func (c *Compiler) buildDetectionEngineExecutionStep(data *WorkflowData) []strin
 	}
 	threatDetectionData.Model = resolvedDetectionModel
 	threatDetectionData.EngineConfig = detectionEngineConfig
-	threatDetectionData.ModelMappings = data.ModelMappings // propagate alias map so detection awf-config.json can resolve model aliases
 	var detectionFirewall *FirewallConfig
 	if threatDetectionData.NetworkPermissions != nil {
 		detectionFirewall = threatDetectionData.NetworkPermissions.Firewall
 	}
 	threatDetectionData.NetworkPermissions = &NetworkPermissions{
-		Allowed:  getThreatDetectionAdditionalAllowedDomains(data),
+		Allowed:  getThreatDetectionAdditionalAllowedDomains(data, engineSetting),
 		Firewall: detectionFirewall,
 	}