Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/minor-rename-llmgateway-port.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 7 additions & 12 deletions pkg/workflow/agentic_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,6 @@ type CapabilityProvider interface {
// SupportsMaxContinuations returns true if this engine supports the max-continuations feature
// When true, max-continuations > 1 enables autopilot/multi-run mode for the engine
SupportsMaxContinuations() bool

// SupportsLLMGateway returns the LLM gateway port number for this engine
// Returns the port number (e.g., 10000) if the engine supports an LLM gateway
// Returns -1 if the engine does not support an LLM gateway
// The port is used to configure AWF api-proxy sidecar container
// In strict mode, engines without LLM gateway support require additional security constraints
SupportsLLMGateway() int
}

// WorkflowExecutor handles workflow compilation and execution
Expand Down Expand Up @@ -226,7 +219,7 @@ type BaseEngine struct {
supportsWebFetch bool
supportsWebSearch bool
supportsPlugins bool
supportsLLMGateway bool
llmGatewayPort int
}

func (e *BaseEngine) GetID() string {
Expand Down Expand Up @@ -269,10 +262,8 @@ func (e *BaseEngine) SupportsMaxContinuations() bool {
return e.supportsMaxContinuations
}

func (e *BaseEngine) SupportsLLMGateway() int {
// Engines that support LLM gateway must override this method
// to return their specific port number (e.g., 10000, 10001, 10002)
return -1
func (e *BaseEngine) getLLMGatewayPort() int {
return e.llmGatewayPort
}

// GetDeclaredOutputFiles returns an empty list by default (engines can override)
Expand Down Expand Up @@ -381,6 +372,10 @@ func GetGlobalEngineRegistry() *EngineRegistry {

// Register adds an engine to the registry
func (r *EngineRegistry) Register(engine CodingAgentEngine) {
type portProvider interface{ getLLMGatewayPort() int }
if p, ok := engine.(portProvider); ok && p.getLLMGatewayPort() < 0 {
panic(fmt.Sprintf("engine '%s': llmGatewayPort must be >= 0, got %d", engine.GetID(), p.getLLMGatewayPort()))
}
Comment on lines 374 to +378
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Register validates llmGatewayPort only when the engine satisfies an interface with an unexported method (getLLMGatewayPort). Engines implemented outside the workflow package can’t implement that method, so the assertion will always fail and the validation will be skipped. If llmGatewayPort is mandatory, consider making the port accessor exported (or part of a public optional interface) and/or treating “missing port provider” as an error so invalid/misconfigured engines can’t be registered silently.

This issue also appears on line 375 of the same file.

Copilot uses AI. Check for mistakes.
agenticEngineLog.Printf("Registering engine: id=%s, name=%s", engine.GetID(), engine.GetDisplayName())
r.engines[engine.GetID()] = engine
}
Expand Down
21 changes: 7 additions & 14 deletions pkg/workflow/awf_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ type AWFCommandConfig struct {
// UsesTTY indicates if the engine requires a TTY (e.g., Claude)
UsesTTY bool

// UsesAPIProxy indicates if the engine uses LLM API proxy
UsesAPIProxy bool

// AllowedDomains is the comma-separated list of allowed domains
AllowedDomains string

Expand Down Expand Up @@ -175,12 +172,10 @@ func BuildAWFArgs(config AWFCommandConfig) []string {
awfArgs = append(awfArgs, "--log-level", awfLogLevel)
awfArgs = append(awfArgs, "--proxy-logs-dir", string(constants.AWFProxyLogsDir))

// Add --enable-host-access when MCP servers are configured (gateway is used)
// OR when the API proxy sidecar is enabled (needs to reach host.docker.internal:<port>)
if HasMCPServers(config.WorkflowData) || config.UsesAPIProxy {
awfArgs = append(awfArgs, "--enable-host-access")
awfHelpersLog.Print("Added --enable-host-access for MCP gateway communication")
}
// Always add --enable-host-access: needed for the API proxy sidecar
// (to reach host.docker.internal:<port>) and for MCP gateway communication
awfArgs = append(awfArgs, "--enable-host-access")
awfHelpersLog.Print("Added --enable-host-access for API proxy and MCP gateway")

// Pin AWF Docker image version to match the installed binary version
awfImageTag := getAWFImageTag(firewallConfig)
Expand All @@ -191,11 +186,9 @@ func BuildAWFArgs(config AWFCommandConfig) []string {
awfArgs = append(awfArgs, "--skip-pull")
awfHelpersLog.Print("Using --skip-pull since images are pre-downloaded")

// Enable API proxy sidecar if needed
if config.UsesAPIProxy {
awfArgs = append(awfArgs, "--enable-api-proxy")
awfHelpersLog.Print("Added --enable-api-proxy for LLM API proxying")
}
// Enable API proxy sidecar (always required for LLM gateway)
awfArgs = append(awfArgs, "--enable-api-proxy")
awfHelpersLog.Print("Added --enable-api-proxy for LLM API proxying")

// Add SSL Bump support for HTTPS content inspection (v0.9.0+)
sslBumpArgs := getSSLBumpArgs(firewallConfig)
Expand Down
18 changes: 4 additions & 14 deletions pkg/workflow/claude_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,14 @@ func NewClaudeEngine() *ClaudeEngine {
description: "Uses Claude Code with full MCP tool support and allow-listing",
experimental: false,
supportsToolsAllowlist: true,
supportsMaxTurns: true, // Claude supports max-turns feature
supportsWebFetch: true, // Claude has built-in WebFetch support
supportsWebSearch: true, // Claude has built-in WebSearch support
supportsLLMGateway: false, // Claude does not support LLM gateway
supportsMaxTurns: true, // Claude supports max-turns feature
supportsWebFetch: true, // Claude has built-in WebFetch support
supportsWebSearch: true, // Claude has built-in WebSearch support
llmGatewayPort: constants.ClaudeLLMGatewayPort,
},
}
}

// SupportsLLMGateway returns the LLM gateway port for Claude engine
func (e *ClaudeEngine) SupportsLLMGateway() int {
return constants.ClaudeLLMGatewayPort
}

// GetModelEnvVarName returns the native environment variable name that the Claude Code CLI uses
// for model selection. Setting ANTHROPIC_MODEL is equivalent to passing --model to the CLI.
func (e *ClaudeEngine) GetModelEnvVarName() string {
Expand Down Expand Up @@ -280,10 +275,6 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
// Get allowed domains (Claude defaults + network permissions + HTTP MCP server URLs + runtime ecosystem domains)
allowedDomains := GetClaudeAllowedDomainsWithToolsAndRuntimes(workflowData.NetworkPermissions, workflowData.Tools, workflowData.Runtimes)

// Enable API proxy sidecar if this engine supports LLM gateway
llmGatewayPort := e.SupportsLLMGateway()
usesAPIProxy := llmGatewayPort > 0

// Build AWF command with all configuration
// AWF v0.15.0+ uses chroot mode by default, providing transparent access to host binaries
// AWF with --enable-chroot and --env-all handles most PATH setup natively:
Expand All @@ -302,7 +293,6 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
LogFile: logFile,
WorkflowData: workflowData,
UsesTTY: true, // Claude Code CLI requires TTY
UsesAPIProxy: usesAPIProxy,
AllowedDomains: allowedDomains,
PathSetup: promptSetup, // Prompt setup runs BEFORE AWF on the host
})
Expand Down
12 changes: 1 addition & 11 deletions pkg/workflow/codex_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,11 @@ func NewCodexEngine() *CodexEngine {
supportsMaxTurns: false, // Codex does not support max-turns feature
supportsWebFetch: false, // Codex does not have built-in web-fetch support
supportsWebSearch: true, // Codex has built-in web-search support
supportsLLMGateway: true, // Codex supports LLM gateway on port 10001
llmGatewayPort: constants.CodexLLMGatewayPort,
},
}
}

// SupportsLLMGateway returns the LLM gateway port for Codex engine
func (e *CodexEngine) SupportsLLMGateway() int {
return constants.CodexLLMGatewayPort
}

// GetModelEnvVarName returns an empty string because the Codex CLI does not support
// selecting the model via a native environment variable. Model selection for Codex
// is done via the -c model=... configuration override in the shell command.
Expand Down Expand Up @@ -206,10 +201,6 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri
// Get allowed domains (Codex defaults + network permissions + HTTP MCP server URLs + runtime ecosystem domains)
allowedDomains := GetCodexAllowedDomainsWithToolsAndRuntimes(workflowData.NetworkPermissions, workflowData.Tools, workflowData.Runtimes)

// Enable API proxy sidecar if this engine supports LLM gateway
llmGatewayPort := e.SupportsLLMGateway()
usesAPIProxy := llmGatewayPort > 0

// Build the command with agent file handling if specified
// INSTRUCTION reading is done inside the AWF command to avoid Docker Compose interpolation
// issues with $ characters in the prompt.
Expand All @@ -236,7 +227,6 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri
LogFile: logFile,
WorkflowData: workflowData,
UsesTTY: false, // Codex is not a TUI, outputs to stdout/stderr
UsesAPIProxy: usesAPIProxy,
AllowedDomains: allowedDomains,
PathSetup: "mkdir -p \"$CODEX_HOME/logs\"", // Create logs directory before AWF
})
Expand Down
7 changes: 1 addition & 6 deletions pkg/workflow/copilot_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewCopilotEngine() *CopilotEngine {
supportsWebFetch: true, // Copilot CLI has built-in web-fetch support
supportsWebSearch: false, // Copilot CLI does not have built-in web-search support
supportsPlugins: true, // Copilot supports plugin installation
supportsLLMGateway: true, // Copilot supports LLM gateway on port 10003
llmGatewayPort: constants.CopilotLLMGatewayPort,
},
}
}
Expand All @@ -61,11 +61,6 @@ func (e *CopilotEngine) GetModelEnvVarName() string {
return constants.CopilotCLIModelEnvVar
}

// SupportsLLMGateway returns the LLM gateway port for Copilot engine
func (e *CopilotEngine) SupportsLLMGateway() int {
return constants.CopilotLLMGatewayPort
}

// GetRequiredSecretNames returns the list of secrets required by the Copilot engine
// This includes COPILOT_GITHUB_TOKEN and optionally MCP_GATEWAY_API_KEY
func (e *CopilotEngine) GetRequiredSecretNames(workflowData *WorkflowData) []string {
Expand Down
6 changes: 0 additions & 6 deletions pkg/workflow/copilot_engine_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,6 @@ func (e *CopilotEngine) GetExecutionSteps(workflowData *WorkflowData, logFile st
// Get allowed domains (copilot defaults + network permissions + HTTP MCP server URLs + runtime ecosystem domains)
allowedDomains := GetCopilotAllowedDomainsWithToolsAndRuntimes(workflowData.NetworkPermissions, workflowData.Tools, workflowData.Runtimes)

// Build AWF command with all configuration
// Enable API proxy sidecar if this engine supports LLM gateway
llmGatewayPort := e.SupportsLLMGateway()
usesAPIProxy := llmGatewayPort > 0

// AWF v0.15.0+ uses chroot mode by default, providing transparent access to host binaries
// AWF v0.15.0+ with --env-all handles PATH natively (chroot mode is default):
// 1. Captures host PATH → AWF_HOST_PATH (already has correct ordering from actions/setup-*)
Expand All @@ -222,7 +217,6 @@ func (e *CopilotEngine) GetExecutionSteps(workflowData *WorkflowData, logFile st
LogFile: logFile,
WorkflowData: workflowData,
UsesTTY: false, // Copilot doesn't require TTY
UsesAPIProxy: usesAPIProxy,
AllowedDomains: allowedDomains,
PathSetup: "", // No path setup needed on host side
})
Expand Down
17 changes: 6 additions & 11 deletions pkg/workflow/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,15 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio
dockerLog.Printf("Added AWF agent container: %s", agentImage)
}

// Add api-proxy sidecar container for engines that support LLM gateway
// Add api-proxy sidecar container (required for all engines LLM gateway is mandatory)
// The api-proxy holds LLM API keys securely and proxies requests through Squid
// Each engine uses its own dedicated port for communication
// Check if the engine supports LLM gateway by querying the engine registry
if workflowData != nil && workflowData.AI != "" {
registry := GetGlobalEngineRegistry()
engine, err := registry.GetEngine(workflowData.AI)
if err == nil && engine.SupportsLLMGateway() > 0 {
apiProxyImage := constants.DefaultFirewallRegistry + "/api-proxy:" + awfImageTag
if !imageSet[apiProxyImage] {
images = append(images, apiProxyImage)
imageSet[apiProxyImage] = true
dockerLog.Printf("Added AWF api-proxy sidecar container for engine with LLM gateway support: %s", apiProxyImage)
}
apiProxyImage := constants.DefaultFirewallRegistry + "/api-proxy:" + awfImageTag
if !imageSet[apiProxyImage] {
images = append(images, apiProxyImage)
imageSet[apiProxyImage] = true
dockerLog.Printf("Added AWF api-proxy sidecar container: %s", apiProxyImage)
}
}
Comment on lines 111 to 118
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collectDockerImages only adds the api-proxy sidecar image when workflowData.AI != "". However, workflows that rely on the default engine can legitimately have AI == "" (the compiler falls back to GetDefaultEngine()), while AWF args now always include --enable-api-proxy and --skip-pull. In that case the workflow can fail at runtime because the required api-proxy image was never pre-downloaded. Consider removing the workflowData.AI != "" guard (or basing the decision on whether AWF will be invoked) so api-proxy is always included whenever the firewall/AWF is enabled.

See below for a potential fix:

		apiProxyImage := constants.DefaultFirewallRegistry + "/api-proxy:" + awfImageTag
		if !imageSet[apiProxyImage] {
			images = append(images, apiProxyImage)
			imageSet[apiProxyImage] = true
			dockerLog.Printf("Added AWF api-proxy sidecar container: %s", apiProxyImage)

Copilot uses AI. Check for mistakes.
}
Expand Down
12 changes: 1 addition & 11 deletions pkg/workflow/gemini_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,11 @@ func NewGeminiEngine() *GeminiEngine {
supportsWebFetch: false,
supportsWebSearch: false,
supportsPlugins: false,
supportsLLMGateway: true, // Gemini supports LLM gateway on port 10003
llmGatewayPort: constants.GeminiLLMGatewayPort,
},
}
}

// SupportsLLMGateway returns the LLM gateway port for Gemini engine
func (e *GeminiEngine) SupportsLLMGateway() int {
return constants.GeminiLLMGatewayPort
}

// GetModelEnvVarName returns the native environment variable name that the Gemini CLI uses
// for model selection. Setting GEMINI_MODEL is equivalent to passing --model to the CLI.
func (e *GeminiEngine) GetModelEnvVarName() string {
Expand Down Expand Up @@ -233,17 +228,12 @@ func (e *GeminiEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
npmPathSetup := GetNpmBinPathSetup()
geminiCommandWithPath := fmt.Sprintf("%s && %s", npmPathSetup, geminiCommand)

// Enable API proxy sidecar if this engine supports LLM gateway
llmGatewayPort := e.SupportsLLMGateway()
usesAPIProxy := llmGatewayPort > 0

command = BuildAWFCommand(AWFCommandConfig{
EngineName: "gemini",
EngineCommand: geminiCommandWithPath,
LogFile: logFile,
WorkflowData: workflowData,
UsesTTY: false,
UsesAPIProxy: usesAPIProxy,
AllowedDomains: allowedDomains,
})
} else {
Expand Down
1 change: 0 additions & 1 deletion pkg/workflow/gemini_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ func TestGeminiEngine(t *testing.T) {
assert.False(t, engine.SupportsWebFetch(), "Should not support built-in web fetch")
assert.False(t, engine.SupportsWebSearch(), "Should not support built-in web search")
assert.False(t, engine.SupportsPlugins(), "Should not support plugins")
assert.Equal(t, 10003, engine.SupportsLLMGateway(), "Should support LLM gateway on port 10003")
})

t.Run("required secrets", func(t *testing.T) {
Expand Down
56 changes: 3 additions & 53 deletions pkg/workflow/strict_mode_llm_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ package workflow
import (
"strings"
"testing"

"github.com/github/gh-aw/pkg/constants"
)

// TestValidateStrictFirewall_LLMGatewaySupport tests the LLM gateway validation in strict mode
Expand Down Expand Up @@ -176,7 +174,7 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) {
}
})

t.Run("copilot engine with LLM gateway support requires sandbox.agent to be enabled in strict mode", func(t *testing.T) {
t.Run("copilot engine requires sandbox.agent to be enabled in strict mode", func(t *testing.T) {
compiler := NewCompiler()
compiler.strictMode = true

Expand All @@ -194,8 +192,7 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) {
if err == nil {
t.Error("Expected error for copilot engine with sandbox.agent: false, got nil")
}
// Since copilot now supports LLM gateway, it should get the general error message
// (not the engine-specific "does not support LLM gateway" message)
// All engines use the general error message now (LLM gateway is always present)
if err != nil && !strings.Contains(err.Error(), "sandbox.agent: false") {
t.Errorf("Expected error about sandbox.agent: false, got: %v", err)
}
Expand All @@ -204,7 +201,7 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) {
}
})

t.Run("codex engine with LLM gateway rejects sandbox.agent: false in strict mode", func(t *testing.T) {
t.Run("codex engine rejects sandbox.agent: false in strict mode", func(t *testing.T) {
compiler := NewCompiler()
compiler.strictMode = true

Expand Down Expand Up @@ -263,53 +260,6 @@ func TestValidateStrictFirewall_LLMGatewaySupport(t *testing.T) {
})
}

// TestSupportsLLMGateway tests the SupportsLLMGateway method for each engine
func TestSupportsLLMGateway(t *testing.T) {
registry := NewEngineRegistry()

tests := []struct {
engineID string
expectedPort int
description string
}{
{
engineID: "codex",
expectedPort: constants.CodexLLMGatewayPort,
description: "Codex engine uses dedicated port for LLM gateway",
},
{
engineID: "claude",
expectedPort: constants.ClaudeLLMGatewayPort,
description: "Claude engine uses dedicated port for LLM gateway",
},
{
engineID: "copilot",
expectedPort: constants.CopilotLLMGatewayPort,
description: "Copilot engine uses dedicated port for LLM gateway",
},
{
engineID: "gemini",
expectedPort: constants.GeminiLLMGatewayPort,
description: "Gemini engine uses dedicated port for LLM gateway",
},
}

for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
engine, err := registry.GetEngine(tt.engineID)
if err != nil {
t.Fatalf("Failed to get engine '%s': %v", tt.engineID, err)
}

llmGatewayPort := engine.SupportsLLMGateway()
if llmGatewayPort != tt.expectedPort {
t.Errorf("Engine '%s': expected SupportsLLMGateway() = %d, got %d",
tt.engineID, tt.expectedPort, llmGatewayPort)
}
})
}
}

// TestValidateStrictFirewall_EcosystemSuggestions tests ecosystem suggestions in warning messages
func TestValidateStrictFirewall_EcosystemSuggestions(t *testing.T) {
t.Run("warns with ecosystem suggestion when individual domain from ecosystem is used", func(t *testing.T) {
Expand Down
Loading
Loading