diff --git a/docs/adr/69364-propagate-private-ca-cert-to-threat-detection.md b/docs/adr/69364-propagate-private-ca-cert-to-threat-detection.md new file mode 100644 index 00000000000..15663914eb8 --- /dev/null +++ b/docs/adr/69364-propagate-private-ca-cert-to-threat-detection.md @@ -0,0 +1,58 @@ +# ADR-69364: Propagate Private CA Certificates to the Threat-Detection Job + +**Date**: 2026-10-16 +**Status**: Draft (generated by the Design Decision Gate for PR #69364; not yet accepted) +**Deciders**: Not recorded; pending maintainer review of PR #69364. +**Author**: PR #69364 (`copilot/fix-ca-cert-propagation`), issue #69015 + +## Context + +`sandbox.agent.ca-cert` is translated into AWF's `apiProxy.caCert` for the agent job, so an engine that talks to an upstream gateway with a private-CA certificate works there. `buildThreatDetectionWorkflowData` in `pkg/workflow/threat_detection_external.go` rebuilt the detection sandbox from scratch (`Type`, `Images` only), so the CA path was dropped. The detection job inherits the same engine id and `engine.env` and therefore calls the same upstream through its own api-proxy sidecar, which does not trust the private CA. TLS verification fails, detection fails, and safe outputs do not run (issue #69015). There was also no frontmatter key to set a certificate for detection alone, and detection runs on a *fresh* runner, so any certificate file written for the agent job does not exist there. + +## Decision + +Detection resolves its API-proxy CA certificate through a single helper, `resolveThreatDetectionCACert`: + +- A non-empty `safe-outputs.threat-detection.ca-cert` wins. +- Otherwise the detection sandbox inherits `sandbox.agent.ca-cert` (via the existing `extractAPIProxyCACert`). +- Unset or empty values inherit; the parser trims whitespace so `" "` is equivalent to unset, and `caCert` stays omitted from the AWF config when nothing is configured. + +The new key is a plain string in the main workflow schema (`ca-cert`), parsed into `ThreatDetectionConfig.CACert`, and the detection sandbox is built as a distinct value so mutating it cannot affect the agent sandbox. The compiler deliberately does **not** copy or materialize the certificate file: documentation states that the file must be created on the detection runner through `threat-detection.steps`. + +The primary driver is correctness with the least new surface — reuse the agent's existing CA plumbing instead of adding a second certificate mechanism — while still allowing a detection-specific override for workflows whose detection engine targets a different upstream. + +## Alternatives Considered + +### Inherit `sandbox.agent.ca-cert` only, with no override key + +This is the minimal fix for #69015 and needs no schema change. It was rejected because detection may legitimately point at a different gateway (or a different trust anchor) than the agent, and without an override the only workarounds remain "disable threat detection" or "override every inherited `engine.env` key". + +### Have the compiler copy/emit the certificate into the detection job automatically + +Generating a step that re-materializes the certificate on the detection runner would remove the documented manual `threat-detection.steps` requirement. It was rejected because the certificate content normally lives in a secret the compiler must not read or inline, and emitting secret-bearing steps implicitly would widen the compiler's security responsibility. Explicit pre-steps keep secret handling in the workflow author's hands. + +### Configure the certificate under `threat-detection.engine` instead + +Issue #69015 mentioned this as an option. It was rejected because the CA path is a sandbox/proxy concern, not an engine concern; placing it on the detection config keeps it symmetric with `sandbox.agent.ca-cert` and avoids overloading engine configuration semantics. + +## Consequences + +### Positive + +- Workflows behind a private CA no longer fail in detection while succeeding in the agent job; safe outputs run as expected. +- Detection can target a different upstream than the agent via an explicit, documented override, without touching inherited `engine.env`. +- Inheritance reuses `extractAPIProxyCACert`, so there is one resolution path for `apiProxy.caCert` rather than two divergent ones. + +### Negative + +- Configuration still fails at runtime (not compile time) if the author forgets the `threat-detection.steps` that writes the certificate on the fresh detection runner; the compiler cannot validate host paths. +- Another frontmatter key expands the `safe-outputs.threat-detection` surface that must be documented, schema-validated, and kept in sync with AWF behaviour (requires AWF v0.28.10+). + +### Neutral + +- Empty/whitespace values are normalized to "inherit" rather than "disable"; there is intentionally no way to suppress an inherited certificate for detection. +- Coverage added at three levels — schema validation, config parsing, and emitted AWF config for inline and external detection across `claude`/`codex`/`copilot` — including an assertion that the detection sandbox does not mutate the agent sandbox. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/docs/src/content/docs/reference/threat-detection.md b/docs/src/content/docs/reference/threat-detection.md index 63cb778b782..ea98aa3c893 100644 --- a/docs/src/content/docs/reference/threat-detection.md +++ b/docs/src/content/docs/reference/threat-detection.md @@ -102,6 +102,7 @@ safe-outputs: | `enabled` | boolean | Enable or disable detection (default: `true` when safe-outputs exist) | | `prompt` | string | Custom instructions appended to default detection prompt | | `engine` | string/object/false | AI engine config (`"copilot"`, full config object, or `false` for no AI) | +| `ca-cert` | string/false | Absolute host path to a CA certificate for the detection API proxy. Inherits `sandbox.agent.ca-cert` when unset or empty and the detection engine matches the agent engine; set to `false` to disable inheritance. Requires AWF v0.28.10 or later. | | `runs-on` | string/array/object | Runner for the detection job (default: inherits from workflow `runs-on`) | | `artifact-base-url` | string | HTTPS base URL for a mirror of the pinned detector release assets. The compiler appends the pinned release tag and asset name; the embedded digest cannot be overridden. | | `steps` | array | Additional GitHub Actions steps to run **before** AI analysis (pre-steps) | @@ -110,6 +111,26 @@ safe-outputs: | `continue-on-error` | boolean | When `true` (default), detection warnings/failures produce a caution notice instead of blocking safe outputs. | | `report-as-issue` | boolean | When `true` (default), detection warnings/failures create or update the `[aw] Detection Runs` tracking issue. Set to `false` to keep threat detection and its enforcement enabled while skipping the tracking issue; results remain visible in the GitHub Actions run logs. | +### Private CA certificates + +The detection job inherits `sandbox.agent.ca-cert` for upstream API proxy TLS verification only when the detection and agent engines match. A non-empty `safe-outputs.threat-detection.ca-cert` overrides the inherited path; set it to `false` to opt out, or leave it unset/empty to use the matching-engine inheritance behavior. + +Detection runs on a fresh runner, so the certificate file must also exist there before engine execution. Use detection pre-steps to write it: + +```yaml wrap +safe-outputs: + create-issue: + threat-detection: + ca-cert: /tmp/detection-ca.pem + steps: + - name: Write CA certificate + env: + CA_PEM: ${{ secrets.INTERNAL_CA_PEM }} + run: printf '%s\n' "$CA_PEM" > /tmp/detection-ca.pem +``` + +AWF versions before v0.28.10 silently omit `apiProxy.caCert` from the configuration instead of failing compilation. The custom certificate is therefore not used, and connections that require it may fail TLS verification. + ### Use an artifact mirror Set `artifact-base-url` when detector binaries must be downloaded through an diff --git a/pkg/parser/schema_threat_detection_test.go b/pkg/parser/schema_threat_detection_test.go index 2dfca4482be..5d9ca1036a3 100644 --- a/pkg/parser/schema_threat_detection_test.go +++ b/pkg/parser/schema_threat_detection_test.go @@ -4,6 +4,36 @@ package parser import "testing" +func TestMainWorkflowSchemaThreatDetectionCACert(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + name string + value any + valid bool + }{ + {name: "absolute certificate path", value: "/tmp/detection-ca.pem", valid: true}, + {name: "empty path", value: "", valid: true}, + {name: "explicitly disable inheritance", value: false, valid: true}, + {name: "relative path rejected", value: "tmp/detection-ca.pem"}, + {name: "boolean rejected", value: true}, + {name: "number rejected", value: 42}, + } { + t.Run(tt.name, func(t *testing.T) { + err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(map[string]any{ + "on": "push", + "engine": "claude", + "safe-outputs": map[string]any{ + "create-issue": nil, + "threat-detection": map[string]any{"ca-cert": tt.value}, + }, + }, "/tmp/gh-aw/threat-detection-ca-cert.md") + if (err == nil) != tt.valid { + t.Errorf("validation error = %v, want valid=%v", err, tt.valid) + } + }) + } +} + func TestMainWorkflowSchemaThreatDetectionArtifactBaseURL(t *testing.T) { t.Parallel() diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 62fd4194faa..ea835d9309f 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -12537,6 +12537,19 @@ "type": "string", "description": "GitHub Actions environment override for the detection job." }, + "ca-cert": { + "oneOf": [ + { + "type": "string", + "pattern": "^(/|$)" + }, + { + "const": false + } + ], + "description": "Absolute host path to an additional CA certificate for the detection job's API proxy upstream TLS verification. Defaults to sandbox.agent.ca-cert when unset or empty and the detection engine matches the agent engine; set to false to disable inheritance. The file must exist on the detection runner before engine execution; use threat-detection.steps to create it. Requires AWF v0.28.10 or later.", + "examples": ["/tmp/detection-ca.pem", false] + }, "artifact-base-url": { "type": "string", "format": "uri", diff --git a/pkg/workflow/threat_detection_config.go b/pkg/workflow/threat_detection_config.go index 4d885db1020..118e3cc5bc1 100644 --- a/pkg/workflow/threat_detection_config.go +++ b/pkg/workflow/threat_detection_config.go @@ -16,6 +16,8 @@ type ThreatDetectionConfig struct { Retries *int `yaml:"retries,omitempty"` // Detector-only retries override passed to threat-detect (--retries) MaxAICredits int64 `yaml:"max-ai-credits,omitempty"` // Maximum AI credits budget for threat-detection engine execution Model string `yaml:"model,omitempty"` // Model override for threat detection engine execution + CACert string `yaml:"ca-cert,omitempty"` // Host CA certificate path override (inherits the agent path for matching engines) + DisableCACert bool `yaml:"-"` // Explicitly disable inheriting sandbox.agent.ca-cert EngineConfig *EngineConfig `yaml:"engine-config,omitempty"` // Extended engine configuration for threat detection EngineDisabled bool `yaml:"-"` // Internal flag: true when engine is explicitly set to false RunsOn string `yaml:"runs-on,omitempty"` // Runner override for the detection job @@ -192,11 +194,20 @@ func parseThreatDetectionScalarFields(configMap map[string]any, threatConfig *Th threatConfig.RunsOn = renderRunsOnSnippet(runOn) } + parseThreatDetectionNetworkFields(configMap, threatConfig) + parseThreatDetectionReportingFields(configMap, threatConfig) +} + +func parseThreatDetectionNetworkFields(configMap map[string]any, threatConfig *ThreatDetectionConfig) { if artifactBaseURL, ok := configMap["artifact-base-url"].(string); ok { threatConfig.ArtifactBaseURL = artifactBaseURL } - parseThreatDetectionReportingFields(configMap, threatConfig) + if caCert, ok := configMap["ca-cert"].(string); ok { + threatConfig.CACert = strings.TrimSpace(caCert) + } else if caCert, ok := configMap["ca-cert"].(bool); ok { + threatConfig.DisableCACert = !caCert + } } func parseThreatDetectionModel(configMap map[string]any, threatConfig *ThreatDetectionConfig) { diff --git a/pkg/workflow/threat_detection_config_test.go b/pkg/workflow/threat_detection_config_test.go index 5ae3b823b95..9867730752b 100644 --- a/pkg/workflow/threat_detection_config_test.go +++ b/pkg/workflow/threat_detection_config_test.go @@ -353,6 +353,34 @@ func TestParseThreatDetectionConfig(t *testing.T) { } } +func TestParseThreatDetectionConfigCACert(t *testing.T) { + tests := []struct { + name string + value any + want string + disableInherit bool + }{ + {name: "path", value: "/tmp/detection-ca.pem", want: "/tmp/detection-ca.pem"}, + {name: "trimmed path", value: " /tmp/detection-ca.pem \n", want: "/tmp/detection-ca.pem"}, + {name: "empty inherits", value: "", want: ""}, + {name: "whitespace inherits", value: " \t", want: ""}, + {name: "false disables inheritance", value: false, disableInherit: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := NewCompiler().parseThreatDetectionConfig(map[string]any{ + "threat-detection": map[string]any{"ca-cert": tt.value}, + }) + if config.CACert != tt.want { + t.Errorf("CA certificate = %q, want %q", config.CACert, tt.want) + } + if config.DisableCACert != tt.disableInherit { + t.Errorf("DisableCACert = %v, want %v", config.DisableCACert, tt.disableInherit) + } + }) + } +} + func TestIsContinueOnError(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/threat_detection_external.go b/pkg/workflow/threat_detection_external.go index 3539cdd7dd0..308fd46f011 100644 --- a/pkg/workflow/threat_detection_external.go +++ b/pkg/workflow/threat_detection_external.go @@ -137,6 +137,7 @@ func buildThreatDetectionWorkflowData(data *WorkflowData, engineID string) *Work Agent: &AgentSandboxConfig{ Type: SandboxTypeAWF, Images: maps.Clone(getSandboxAgentImages(data)), + CACert: resolveThreatDetectionCACert(data, engineID), }, }, } @@ -171,6 +172,21 @@ func buildThreatDetectionWorkflowData(data *WorkflowData, engineID string) *Work return detectionData } +// resolveThreatDetectionCACert prefers an explicit detection CA path, honors +// ca-cert: false, and otherwise inherits the agent path only for the same engine. +func resolveThreatDetectionCACert(data *WorkflowData, engineID string) string { + if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.CACert != "" { + return data.SafeOutputs.ThreatDetection.CACert + } + if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.DisableCACert { + return "" + } + if ResolveEngineID(data) != engineID { + return "" + } + return extractAPIProxyCACert(data) +} + // 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. diff --git a/pkg/workflow/threat_detection_job_runtime_test.go b/pkg/workflow/threat_detection_job_runtime_test.go index 549ca093d58..90e62a11b9a 100644 --- a/pkg/workflow/threat_detection_job_runtime_test.go +++ b/pkg/workflow/threat_detection_job_runtime_test.go @@ -349,6 +349,182 @@ func TestBuildPullAWFContainersStepPropagatesGitHubCLIMode(t *testing.T) { }) } +func TestBuildThreatDetectionAWFConfigCACert(t *testing.T) { + tests := []struct { + name string + sandbox *SandboxConfig + safe *SafeOutputsConfig + detectionEngine string + want string + }{ + {name: "no sandbox or safe outputs"}, + {name: "no agent sandbox", sandbox: &SandboxConfig{}}, + {name: "no certificate", sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{}}}, + { + name: "inherits agent certificate", + sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + want: "/tmp/agent-ca.pem", + }, + { + name: "inherits with default detection settings", + sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + safe: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{}}, + want: "/tmp/agent-ca.pem", + }, + { + name: "inherits without detection config", + sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + safe: &SafeOutputsConfig{}, + want: "/tmp/agent-ca.pem", + }, + { + name: "detection override", + sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + safe: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{CACert: "/tmp/detection-ca.pem"}}, + want: "/tmp/detection-ca.pem", + }, + { + name: "explicitly disables inheritance", + sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + safe: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{DisableCACert: true}}, + }, + { + name: "different detection engine does not inherit", + sandbox: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + safe: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{EngineConfig: &EngineConfig{ID: "copilot"}}}, + detectionEngine: "copilot", + }, + { + name: "detection certificate without agent sandbox", + safe: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{CACert: "/tmp/detection-ca.pem"}}, + want: "/tmp/detection-ca.pem", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := &WorkflowData{AI: "claude", SandboxConfig: tt.sandbox, SafeOutputs: tt.safe} + originalCACert := extractAPIProxyCACert(data) + detectionData := buildThreatDetectionWorkflowData(data, tt.detectionEngine) + engineName := tt.detectionEngine + if engineName == "" { + engineName = "claude" + } + if got := detectionData.SandboxConfig.Agent.CACert; got != tt.want { + t.Fatalf("detection CA certificate = %q, want %q", got, tt.want) + } + configJSON, err := BuildAWFConfigJSON(AWFCommandConfig{ + EngineName: engineName, + WorkflowData: detectionData, + }) + if err != nil { + t.Fatalf("BuildAWFConfigJSON() error = %v", err) + } + var config struct { + APIProxy struct { + CACert string `json:"caCert"` + } `json:"apiProxy"` + } + if err := json.Unmarshal([]byte(configJSON), &config); err != nil { + t.Fatalf("unmarshal AWF config: %v", err) + } + if config.APIProxy.CACert != tt.want { + t.Errorf("apiProxy.caCert = %q, want %q", config.APIProxy.CACert, tt.want) + } + if tt.want == "" && strings.Contains(configJSON, `"caCert"`) { + t.Error("apiProxy.caCert must be omitted when not configured") + } + detectionData.SandboxConfig.Agent.CACert = "/tmp/changed.pem" + if extractAPIProxyCACert(data) != originalCACert { + t.Error("detection sandbox must not mutate the agent sandbox") + } + }) + } +} + +func TestDetectionExecutionStepsCACert(t *testing.T) { + compiler := NewCompiler() + for _, engine := range []string{"claude", "codex", "copilot"} { + for _, override := range []string{"", "/tmp/detection-ca.pem"} { + data := &WorkflowData{ + AI: engine, + SandboxConfig: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + SafeOutputs: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{CACert: override}}, + } + want := "/tmp/agent-ca.pem" + if override != "" { + want = override + } + for name, build := range map[string]func(*WorkflowData) []string{ + "external": compiler.buildExternalDetectorExecutionStep, + "inline": compiler.buildDetectionEngineExecutionStep, + } { + t.Run(engine+"/"+name+"/"+want, func(t *testing.T) { + steps := strings.ReplaceAll(strings.Join(build(data), ""), `\"`, `"`) + if !strings.Contains(steps, `"caCert":"`+want+`"`) { + t.Errorf("detection execution steps missing apiProxy.caCert %q", want) + } + if override != "" && strings.Contains(steps, "/tmp/agent-ca.pem") { + t.Error("detection execution steps must not use the overridden agent certificate") + } + }) + } + } + } + + t.Run("different detection engine does not inherit", func(t *testing.T) { + data := &WorkflowData{ + AI: "claude", + SandboxConfig: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + SafeOutputs: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{ + EngineConfig: &EngineConfig{ID: "copilot"}, + }}, + } + 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), "") + if strings.Contains(steps, `"caCert":`) || strings.Contains(steps, "/tmp/agent-ca.pem") { + t.Errorf("different-engine detection must not inherit the agent CA: %s", steps) + } + }) + } + }) + + t.Run("public endpoint override can disable inherited CA", func(t *testing.T) { + data := &WorkflowData{ + AI: "claude", + EngineConfig: &EngineConfig{ + ID: "claude", + APITarget: "private-upstream.example.com", + }, + SandboxConfig: &SandboxConfig{Agent: &AgentSandboxConfig{CACert: "/tmp/agent-ca.pem"}}, + SafeOutputs: &SafeOutputsConfig{ThreatDetection: &ThreatDetectionConfig{ + DisableCACert: true, + EngineConfig: &EngineConfig{ + ID: "claude", + APITarget: "public-upstream.example.com", + }, + }}, + } + 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), "") + if strings.Contains(steps, `"caCert":`) || strings.Contains(steps, "/tmp/agent-ca.pem") { + t.Errorf("public-upstream detection must not inherit the agent CA: %s", steps) + } + if !strings.Contains(steps, "public-upstream.example.com") { + t.Errorf("expected detection to use its public endpoint override: %s", steps) + } + }) + } + }) +} + func TestBuildThreatDetectionWorkflowDataPropagatesImageOverrides(t *testing.T) { images := map[string]string{ awfImageRoleSquid: "mirror.example.com/awf/squid:v1@sha256:" + strings.Repeat("a", 64),