diff --git a/.github/workflows/smoke-call-workflow.lock.yml b/.github/workflows/smoke-call-workflow.lock.yml index 8e6f0b9211f..17814deb639 100644 --- a/.github/workflows/smoke-call-workflow.lock.yml +++ b/.github/workflows/smoke-call-workflow.lock.yml @@ -619,6 +619,11 @@ jobs: "inputSchema": { "additionalProperties": false, "properties": { + "aw_context": { + "default": "", + "description": "Agent caller context (used internally by Agentic Workflows).", + "type": "string" + }, "payload": { "description": "Input parameter 'payload' for workflow smoke-workflow-call", "type": "string" @@ -1102,15 +1107,25 @@ jobs: needs: safe_outputs if: needs.safe_outputs.outputs.call_workflow_name == 'smoke-workflow-call' # Imported from called workflow "smoke-workflow-call" because GitHub requires the caller job to grant permissions requested by reusable workflow jobs. - # Review the called workflow's frontmatter permissions in ./.github/workflows/smoke-workflow-call.md. + # Review the called workflow's job-level permissions in ./.github/workflows/smoke-workflow-call.lock.yml. permissions: + actions: read contents: read - pull-requests: read + issues: write + pull-requests: write uses: ./.github/workflows/smoke-workflow-call.lock.yml with: + aw_context: ${{ fromJSON(needs.safe_outputs.outputs.call_workflow_payload).aw_context }} payload: ${{ needs.safe_outputs.outputs.call_workflow_payload }} task-description: ${{ fromJSON(needs.safe_outputs.outputs.call_workflow_payload)['task-description'] }} - secrets: inherit + secrets: + COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} + GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} + GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} + GH_AW_OTEL_GRAFANA_AUTHORIZATION: ${{ secrets.GH_AW_OTEL_GRAFANA_AUTHORIZATION }} + GH_AW_OTEL_GRAFANA_ENDPOINT: ${{ secrets.GH_AW_OTEL_GRAFANA_ENDPOINT }} + GH_AW_OTEL_SENTRY_AUTHORIZATION: ${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }} + GH_AW_OTEL_SENTRY_ENDPOINT: ${{ secrets.GH_AW_OTEL_SENTRY_ENDPOINT }} conclusion: needs: diff --git a/docs/tests/mobile-responsive.spec.ts b/docs/tests/mobile-responsive.spec.ts index 528648e1d92..5e553403d57 100644 --- a/docs/tests/mobile-responsive.spec.ts +++ b/docs/tests/mobile-responsive.spec.ts @@ -222,7 +222,6 @@ test.describe('Mobile and Responsive Layout', () => { await context.close(); }); - // Regression test for https://github.com/github/gh-aw/issues/29545 // Verify the navigation dropdown is fully within the viewport when large // user fonts cause header elements to shift on Android Chrome. diff --git a/pkg/workflow/threat_detection_external.go b/pkg/workflow/threat_detection_external.go index faa824a4544..5ce1301a706 100644 --- a/pkg/workflow/threat_detection_external.go +++ b/pkg/workflow/threat_detection_external.go @@ -3,6 +3,7 @@ package workflow import ( "fmt" + "slices" "strconv" "strings" @@ -66,28 +67,46 @@ func codexProxyWebsocketBaseURL(apiBase string) string { } } -// buildPullAWFContainersStep creates a step that pre-pulls AWF (agent workflow firewall) -// container images in the detection job. The detection engine runs inside AWF, which uses -// three containers (squid, agent, api-proxy). Pre-pulling avoids on-demand pulls at runtime. -// Only AWF images are pulled here; MCP server images are not needed for detection. -func (c *Compiler) buildPullAWFContainersStep(data *WorkflowData) []string { - // Build a minimal WorkflowData that represents the detection engine context so - // collectDockerImages returns only the AWF firewall images (no MCP tool images). - engineSetting := data.AI - if engineSetting == "" { - engineSetting = "claude" +// buildThreatDetectionWorkflowData creates the shared minimal WorkflowData used by +// detection-job helper steps so topology- and feature-dependent behavior stays in sync. +// It always initializes SandboxConfig.Agent because downstream detection helpers +// extend the agent sandbox configuration (for example, external-detector mounts). +// Callers can pass an empty engineID to inherit the detection job's default engine +// resolution from the source WorkflowData. +func buildThreatDetectionWorkflowData(data *WorkflowData, engineID string) *WorkflowData { + if engineID == "" { + engineID = data.AI } - detectionData := &WorkflowData{ - Tools: map[string]any{}, - AI: engineSetting, + if engineID == "" { + engineID = "claude" + } + + return &WorkflowData{ + AI: engineID, + ActionCache: data.ActionCache, + Features: data.Features, + Permissions: data.Permissions, + CachedPermissions: data.CachedPermissions, + IsDetectionRun: true, + RunnerConfig: data.RunnerConfig, SandboxConfig: &SandboxConfig{ Agent: &AgentSandboxConfig{ Type: SandboxTypeAWF, }, }, - ActionCache: data.ActionCache, // Propagate cache so container digest pins are applied - Features: data.Features, // Propagate features so cli-proxy image is included when enabled } +} + +// buildPullAWFContainersStep creates a step that pre-pulls AWF (agent workflow firewall) +// container images in the detection job. The detection engine runs inside AWF, which uses +// the firewall stack containers needed for the selected topology (squid, agent, api-proxy, +// plus build-tools on arc-dind). Pre-pulling avoids on-demand pulls at runtime. Only AWF +// images are pulled here; MCP server images are not needed for detection. +func (c *Compiler) buildPullAWFContainersStep(data *WorkflowData) []string { + // Build a minimal WorkflowData that represents the detection engine context so + // collectDockerImages returns only the AWF firewall images (no MCP tool images). + detectionData := buildThreatDetectionWorkflowData(data, "") + detectionData.Tools = map[string]any{} images := collectDockerImages(detectionData.Tools, detectionData, c.actionMode) if len(images) == 0 { @@ -179,22 +198,11 @@ func (c *Compiler) buildInstallDetectionEngineForExternalDetectorStep(data *Work // Build a synthetic detection WorkflowData solely to generate the engine's // installation steps for this separate detection job context. - threatDetectionData := &WorkflowData{ - Tools: map[string]any{ - "bash": []any{"*"}, - }, - EngineConfig: &EngineConfig{ID: engineID}, - AI: engineID, - Features: data.Features, - Permissions: data.Permissions, - CachedPermissions: data.CachedPermissions, - IsDetectionRun: true, - SandboxConfig: &SandboxConfig{ - Agent: &AgentSandboxConfig{ - Type: SandboxTypeAWF, - }, - }, + threatDetectionData := buildThreatDetectionWorkflowData(data, engineID) + threatDetectionData.Tools = map[string]any{ + "bash": []any{"*"}, } + threatDetectionData.EngineConfig = &EngineConfig{ID: engineID} if canReuseThreatDetectionEngineConfigForExternalDetector(data, engineID) { ec := data.SafeOutputs.ThreatDetection.EngineConfig @@ -239,6 +247,14 @@ func isAWFBinaryInstallStep(step GitHubActionStep) bool { return false } +func appendThreatDetectionRWMount(mounts []string) []string { + threatDetectionMount := constants.ThreatDetectionDir + ":" + constants.ThreatDetectionDir + ":rw" + if slices.Contains(mounts, threatDetectionMount) { + return mounts + } + return append(mounts, threatDetectionMount) +} + // buildExternalDetectorExecutionStep creates the AWF execution step for the external // threat-detect binary. It runs threat-detect inside the AWF firewall sandbox with a // read-write mount so detection_result.json can be written from inside the container @@ -261,31 +277,18 @@ func (c *Compiler) buildExternalDetectorExecutionStep(data *WorkflowData) []stri // Build detection WorkflowData for the external detector. // The rw mount for ThreatDetectionDir allows the threat-detect binary to write // detection_result.json from inside the AWF container to the host filesystem. - threatDetectionData := &WorkflowData{ - Tools: map[string]any{ - "bash": []any{"*"}, - }, - EngineConfig: &EngineConfig{ID: engineID}, - AI: engineID, - Features: data.Features, - Permissions: data.Permissions, - CachedPermissions: data.CachedPermissions, - IsDetectionRun: true, - NetworkPermissions: &NetworkPermissions{ - Allowed: getThreatDetectionAdditionalAllowedDomains(data), - }, - SandboxConfig: &SandboxConfig{ - Agent: &AgentSandboxConfig{ - Type: SandboxTypeAWF, - // Add a read-write mount so the threat-detect binary can write - // detection_result.json inside the container and it becomes visible - // on the host through the bind mount. - Mounts: []string{ - constants.ThreatDetectionDir + ":" + constants.ThreatDetectionDir + ":rw", - }, - }, - }, - } + threatDetectionData := buildThreatDetectionWorkflowData(data, engineID) + threatDetectionData.Tools = map[string]any{ + "bash": []any{"*"}, + } + threatDetectionData.EngineConfig = &EngineConfig{ID: engineID} + threatDetectionData.NetworkPermissions = &NetworkPermissions{ + Allowed: getThreatDetectionAdditionalAllowedDomains(data), + } + // Add a read-write mount so the threat-detect binary can write + // detection_result.json inside the container and it becomes visible + // on the host through the bind mount. + threatDetectionData.SandboxConfig.Agent.Mounts = appendThreatDetectionRWMount(threatDetectionData.SandboxConfig.Agent.Mounts) // Inherit engine config overrides from threat-detection config when set. if canReuseThreatDetectionEngineConfigForExternalDetector(data, engineID) { diff --git a/pkg/workflow/threat_detection_inline_engine.go b/pkg/workflow/threat_detection_inline_engine.go index dbf3d57f123..443417c058d 100644 --- a/pkg/workflow/threat_detection_inline_engine.go +++ b/pkg/workflow/threat_detection_inline_engine.go @@ -115,33 +115,17 @@ 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. - // RunnerConfig is propagated from the main workflow data so that arc-dind topology - // handling (daemon-visible Copilot staging step + daemon-visible spawn path) applies - // to the detection job the same way it applies to the agent job. // 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 := &WorkflowData{ - Tools: map[string]any{ - "bash": []any{"*"}, - }, - SafeOutputs: nil, - EngineConfig: detectionEngineConfig, - AI: engineSetting, - Features: data.Features, - Permissions: data.Permissions, - CachedPermissions: data.CachedPermissions, - IsDetectionRun: true, // Mark as detection run for phase tagging - RunnerConfig: data.RunnerConfig, // propagate runner.topology (e.g. arc-dind) to the detection job - ModelMappings: data.ModelMappings, // propagate alias map so detection awf-config.json can resolve model aliases - NetworkPermissions: &NetworkPermissions{ - Allowed: getThreatDetectionAdditionalAllowedDomains(data), - }, - SandboxConfig: &SandboxConfig{ - Agent: &AgentSandboxConfig{ - Type: SandboxTypeAWF, - }, - }, + threatDetectionData := buildThreatDetectionWorkflowData(data, engineSetting) + threatDetectionData.Tools = map[string]any{ + "bash": []any{"*"}, + } + threatDetectionData.EngineConfig = detectionEngineConfig + threatDetectionData.ModelMappings = data.ModelMappings // propagate alias map so detection awf-config.json can resolve model aliases + threatDetectionData.NetworkPermissions = &NetworkPermissions{ + Allowed: getThreatDetectionAdditionalAllowedDomains(data), } var steps []string diff --git a/pkg/workflow/threat_detection_test.go b/pkg/workflow/threat_detection_test.go index 7c8b4cb5cf9..d4a0410c671 100644 --- a/pkg/workflow/threat_detection_test.go +++ b/pkg/workflow/threat_detection_test.go @@ -3,6 +3,7 @@ package workflow import ( + "reflect" "strings" "testing" @@ -1961,6 +1962,162 @@ func TestBuildPullAWFContainersStepPropagatesFeatures(t *testing.T) { }) } +func TestBuildPullAWFContainersStepPropagatesRunnerTopology(t *testing.T) { + compiler := NewCompiler() + buildToolsImagePrefix := constants.DefaultFirewallRegistry + "/build-tools:" + + t.Run("arc-dind includes build-tools image", func(t *testing.T) { + data := &WorkflowData{ + AI: "copilot", + RunnerConfig: &RunnerConfig{ + Topology: RunnerTopologyArcDind, + }, + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{}, + }, + } + + steps := compiler.buildPullAWFContainersStep(data) + stepsString := strings.Join(steps, "") + + if !strings.Contains(stepsString, buildToolsImagePrefix) { + t.Errorf("expected build-tools image prefix %q in detection pull step for arc-dind;\ngot:\n%s", buildToolsImagePrefix, stepsString) + } + }) + + t.Run("non-arc-dind excludes build-tools image", func(t *testing.T) { + data := &WorkflowData{ + AI: "copilot", + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{}, + }, + } + + steps := compiler.buildPullAWFContainersStep(data) + stepsString := strings.Join(steps, "") + + if strings.Contains(stepsString, buildToolsImagePrefix) { + t.Errorf("did not expect build-tools image prefix %q in detection pull step without arc-dind;\ngot:\n%s", buildToolsImagePrefix, stepsString) + } + }) + + t.Run("permissions do not change pulled images", func(t *testing.T) { + baseData := &WorkflowData{ + AI: "copilot", + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{}, + }, + } + withPermissions := &WorkflowData{ + AI: baseData.AI, + SafeOutputs: baseData.SafeOutputs, + Permissions: "contents: read", + CachedPermissions: NewPermissionsContentsRead(), + } + + baseSteps := strings.Join(compiler.buildPullAWFContainersStep(baseData), "") + permissionSteps := strings.Join(compiler.buildPullAWFContainersStep(withPermissions), "") + + if permissionSteps != baseSteps { + t.Errorf("expected detection pull step to ignore permissions when collecting images;\nwithout permissions:\n%s\nwith permissions:\n%s", baseSteps, permissionSteps) + } + }) +} + +func TestBuildExternalDetectorExecutionStepPropagatesRunnerTopology(t *testing.T) { + compiler := NewCompiler() + + t.Run("arc-dind uses daemon-visible AWF paths", func(t *testing.T) { + data := &WorkflowData{ + AI: "copilot", + RunnerConfig: &RunnerConfig{ + Topology: RunnerTopologyArcDind, + }, + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{}, + }, + } + + steps := compiler.buildExternalDetectorExecutionStep(data) + if len(steps) == 0 { + t.Fatal("expected non-empty steps") + } + allSteps := strings.Join(steps, "") + + if !strings.Contains(allSteps, `--mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro"`) { + t.Errorf("expected arc-dind external detector execution to mount ${RUNNER_TEMP}/gh-aw read-only;\ngot:\n%s", allSteps) + } + if !strings.Contains(allSteps, `--mount "${RUNNER_TEMP}/gh-aw/home:${RUNNER_TEMP}/gh-aw/home:rw"`) { + t.Errorf("expected arc-dind external detector execution to mount ${RUNNER_TEMP}/gh-aw/home read-write;\ngot:\n%s", allSteps) + } + if !strings.Contains(allSteps, `\"proxyLogsDir\":\"${RUNNER_TEMP}/gh-aw/sandbox/firewall/logs\"`) { + t.Errorf("expected arc-dind external detector execution to rewrite proxyLogsDir under ${RUNNER_TEMP}/gh-aw;\ngot:\n%s", allSteps) + } + if !strings.Contains(allSteps, `\"auditDir\":\"${RUNNER_TEMP}/gh-aw/sandbox/firewall/audit\"`) { + t.Errorf("expected arc-dind external detector execution to rewrite auditDir under ${RUNNER_TEMP}/gh-aw;\ngot:\n%s", allSteps) + } + if !strings.Contains(allSteps, "export HOME=${RUNNER_TEMP}/gh-aw/home") { + t.Errorf("expected arc-dind external detector execution to export HOME under ${RUNNER_TEMP}/gh-aw/home;\ngot:\n%s", allSteps) + } + }) + + t.Run("non-arc-dind keeps standard AWF paths", func(t *testing.T) { + data := &WorkflowData{ + AI: "copilot", + SafeOutputs: &SafeOutputsConfig{ + ThreatDetection: &ThreatDetectionConfig{}, + }, + } + + steps := compiler.buildExternalDetectorExecutionStep(data) + if len(steps) == 0 { + t.Fatal("expected non-empty steps") + } + allSteps := strings.Join(steps, "") + + if strings.Contains(allSteps, `--mount "${RUNNER_TEMP}/gh-aw/home:${RUNNER_TEMP}/gh-aw/home:rw"`) { + t.Errorf("did not expect non-arc-dind external detector execution to mount ${RUNNER_TEMP}/gh-aw/home read-write;\ngot:\n%s", allSteps) + } + if strings.Contains(allSteps, "export HOME=${RUNNER_TEMP}/gh-aw/home") { + t.Errorf("did not expect non-arc-dind external detector execution to export HOME under ${RUNNER_TEMP}/gh-aw/home;\ngot:\n%s", allSteps) + } + if strings.Contains(allSteps, `\"proxyLogsDir\":\"${RUNNER_TEMP}/gh-aw/sandbox/firewall/logs\"`) { + t.Errorf("did not expect non-arc-dind external detector execution to rewrite proxyLogsDir under ${RUNNER_TEMP}/gh-aw;\ngot:\n%s", allSteps) + } + }) +} + +func TestAppendThreatDetectionRWMount(t *testing.T) { + threatDetectionMount := constants.ThreatDetectionDir + ":" + constants.ThreatDetectionDir + ":rw" + + t.Run("appends missing mount without clobbering existing mounts", func(t *testing.T) { + existingMounts := []string{ + "/tmp/existing:/tmp/existing:ro", + "/tmp/other:/tmp/other:rw", + } + + got := appendThreatDetectionRWMount(append([]string(nil), existingMounts...)) + + want := append(append([]string(nil), existingMounts...), threatDetectionMount) + if !reflect.DeepEqual(got, want) { + t.Fatalf("expected mounts %v, got %v", want, got) + } + }) + + t.Run("does not duplicate existing threat-detection mount", func(t *testing.T) { + existingMounts := []string{ + "/tmp/existing:/tmp/existing:ro", + threatDetectionMount, + } + + got := appendThreatDetectionRWMount(append([]string(nil), existingMounts...)) + + if !reflect.DeepEqual(got, existingMounts) { + t.Fatalf("expected mounts %v, got %v", existingMounts, got) + } + }) +} + func TestBuildDetectionEngineExecutionStepEmitsNodeSetupForCopilot(t *testing.T) { compiler := NewCompiler()