diff --git a/.changeset/minor-rename-llmgateway-port.md b/.changeset/minor-rename-llmgateway-port.md new file mode 100644 index 00000000000..0856cb4da92 --- /dev/null +++ b/.changeset/minor-rename-llmgateway-port.md @@ -0,0 +1,4 @@ +--- +"gh-aw": minor +--- +Renamed the agentic workflow engine's `supportsLLMGateway` flag to `llmGatewayPort`, made the gateway port mandatory and validated, removed the `SupportsLLMGateway` interface hooks, and consolidated API proxy/host-access workflow flags. diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index d0fd1ceaf2b..f496b610cfb 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -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 @@ -226,7 +219,7 @@ type BaseEngine struct { supportsWebFetch bool supportsWebSearch bool supportsPlugins bool - supportsLLMGateway bool + llmGatewayPort int } func (e *BaseEngine) GetID() string { @@ -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) @@ -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())) + } agenticEngineLog.Printf("Registering engine: id=%s, name=%s", engine.GetID(), engine.GetDisplayName()) r.engines[engine.GetID()] = engine } diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index af09f9dae03..0de7049a1d6 100644 --- a/pkg/workflow/awf_helpers.go +++ b/pkg/workflow/awf_helpers.go @@ -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 @@ -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:) - 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:) 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) @@ -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) diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 4197be967dc..fa1f82daefb 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -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 { @@ -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: @@ -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 }) diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index d8265dfe76a..32c4f1f0e20 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -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. @@ -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. @@ -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 }) diff --git a/pkg/workflow/copilot_engine.go b/pkg/workflow/copilot_engine.go index 462e6e73803..d317b087857 100644 --- a/pkg/workflow/copilot_engine.go +++ b/pkg/workflow/copilot_engine.go @@ -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, }, } } @@ -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 { diff --git a/pkg/workflow/copilot_engine_execution.go b/pkg/workflow/copilot_engine_execution.go index a7b7ea8d168..a3cbc540dae 100644 --- a/pkg/workflow/copilot_engine_execution.go +++ b/pkg/workflow/copilot_engine_execution.go @@ -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-*) @@ -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 }) diff --git a/pkg/workflow/docker.go b/pkg/workflow/docker.go index e206a72fcf4..1e6b95701c8 100644 --- a/pkg/workflow/docker.go +++ b/pkg/workflow/docker.go @@ -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) } } } diff --git a/pkg/workflow/gemini_engine.go b/pkg/workflow/gemini_engine.go index 563a6c679db..d1c198bf2f7 100644 --- a/pkg/workflow/gemini_engine.go +++ b/pkg/workflow/gemini_engine.go @@ -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 { @@ -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 { diff --git a/pkg/workflow/gemini_engine_test.go b/pkg/workflow/gemini_engine_test.go index a4bf6058fa7..2d69c6d4b17 100644 --- a/pkg/workflow/gemini_engine_test.go +++ b/pkg/workflow/gemini_engine_test.go @@ -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) { diff --git a/pkg/workflow/strict_mode_llm_gateway_test.go b/pkg/workflow/strict_mode_llm_gateway_test.go index 2cdd042e6df..2ff31a97b33 100644 --- a/pkg/workflow/strict_mode_llm_gateway_test.go +++ b/pkg/workflow/strict_mode_llm_gateway_test.go @@ -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 @@ -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 @@ -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) } @@ -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 @@ -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) { diff --git a/pkg/workflow/strict_mode_validation.go b/pkg/workflow/strict_mode_validation.go index 373cad48415..8a257057439 100644 --- a/pkg/workflow/strict_mode_validation.go +++ b/pkg/workflow/strict_mode_validation.go @@ -445,27 +445,12 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N return nil } - // Get the engine instance to check LLM gateway support - agenticEngine, err := c.engineRegistry.GetEngine(engineID) - if err != nil { - strictModeValidationLog.Printf("Failed to get engine: %v", err) - return fmt.Errorf("internal error: failed to get engine '%s': %w", engineID, err) - } - - // Check if engine supports LLM gateway - llmGatewayPort := agenticEngine.SupportsLLMGateway() - strictModeValidationLog.Printf("Engine '%s' LLM gateway port: %d", engineID, llmGatewayPort) - // Check if sandbox.agent: false is set (explicitly disabled) sandboxAgentDisabled := sandboxConfig != nil && sandboxConfig.Agent != nil && sandboxConfig.Agent.Disabled // In strict mode, sandbox.agent: false is not allowed for any engine as it disables the agent sandbox firewall if sandboxAgentDisabled { strictModeValidationLog.Printf("sandbox.agent: false is set, refusing in strict mode") - // For engines without LLM gateway support, provide more specific error message - if llmGatewayPort < 0 { - return fmt.Errorf("strict mode: engine '%s' does not support LLM gateway and requires 'sandbox.agent' to be enabled for security. Remove 'sandbox.agent: false' or set 'strict: false'. See: https://github.github.com/gh-aw/reference/sandbox/", engineID) - } return errors.New("strict mode: 'sandbox.agent: false' is not allowed because it disables the agent sandbox firewall. This removes important security protections. Remove 'sandbox.agent: false' or set 'strict: false' to disable strict mode. See: https://github.github.com/gh-aw/reference/sandbox/") }