diff --git a/actions/setup/sh/install_awf_binary.sh b/actions/setup/sh/install_awf_binary.sh index e95bff73f8b..4be495420c1 100755 --- a/actions/setup/sh/install_awf_binary.sh +++ b/actions/setup/sh/install_awf_binary.sh @@ -6,7 +6,7 @@ # its SHA256 checksum before installation to protect against supply chain attacks. # # Arguments: -# VERSION - AWF version to install (e.g., v0.25.10) +# VERSION - AWF version to install (e.g., v0.25.10) or "latest" # # Install strategy: # 1. If Node.js >= 20 is available, download the lightweight awf-bundle.js (~357KB) @@ -44,7 +44,11 @@ ARCH="$(uname -m)" echo "Installing awf with checksum verification (version: ${AWF_VERSION}, os: ${OS}, arch: ${ARCH})" # Download URLs -BASE_URL="https://github.com/${AWF_REPO}/releases/download/${AWF_VERSION}" +if [ "$AWF_VERSION" = "latest" ]; then + BASE_URL="https://github.com/${AWF_REPO}/releases/latest/download" +else + BASE_URL="https://github.com/${AWF_REPO}/releases/download/${AWF_VERSION}" +fi CHECKSUMS_URL="${BASE_URL}/checksums.txt" # Platform-portable SHA256 function diff --git a/pkg/constants/feature_constants.go b/pkg/constants/feature_constants.go index 73fbc93d146..4e303a4416c 100644 --- a/pkg/constants/feature_constants.go +++ b/pkg/constants/feature_constants.go @@ -12,6 +12,8 @@ type FeatureFlag string // Feature flag identifiers const ( + // NoFeatureFlag indicates that no feature-flag-based override should be applied. + NoFeatureFlag FeatureFlag = "" // MCPScriptsFeatureFlag is the name of the feature flag for mcp-scripts MCPScriptsFeatureFlag FeatureFlag = "mcp-scripts" // MCPGatewayFeatureFlag is the feature flag name for enabling MCP gateway @@ -60,6 +62,66 @@ const ( // features: // byok-copilot: true ByokCopilotFeatureFlag FeatureFlag = "byok-copilot" + // CopilotVersionFeatureFlag overrides the default Copilot CLI version. + // When set to a non-empty string, Copilot CLI installation uses this + // version instead of DefaultCopilotVersion. Supports explicit versions + // (e.g. "1.0.21") and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // copilot-version: "latest" + CopilotVersionFeatureFlag FeatureFlag = "copilot-version" + // MCPGVersionFeatureFlag overrides the default MCP gateway container version. + // When set to a non-empty string, MCP gateway image references use this + // version instead of DefaultMCPGatewayVersion. + // + // Workflow frontmatter usage: + // + // features: + // mcpg-version: "v0.2.27" + MCPGVersionFeatureFlag FeatureFlag = "mcpg-version" + // FirewallVersionFeatureFlag overrides the default firewall container version. + // When set to a non-empty string, AWF installation and image references use + // this version instead of DefaultFirewallVersion. + // + // Workflow frontmatter usage: + // + // features: + // firewall-version: "v0.25.27" + FirewallVersionFeatureFlag FeatureFlag = "firewall-version" + // CodexVersionFeatureFlag overrides the default Codex CLI version. + // Supports explicit versions (e.g. "0.121.0") and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // codex-version: "latest" + CodexVersionFeatureFlag FeatureFlag = "codex-version" + // ClaudeVersionFeatureFlag overrides the default Claude CLI version. + // Supports explicit versions (e.g. "2.1.47") and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // claude-version: "latest" + ClaudeVersionFeatureFlag FeatureFlag = "claude-version" + // OpenCodeVersionFeatureFlag overrides the default OpenCode CLI version. + // Supports explicit versions and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // opencode-version: "latest" + OpenCodeVersionFeatureFlag FeatureFlag = "opencode-version" + // GeminiVersionFeatureFlag overrides the default Gemini CLI version. + // Supports explicit versions and the "latest" tag. + // + // Workflow frontmatter usage: + // + // features: + // gemini-version: "latest" + GeminiVersionFeatureFlag FeatureFlag = "gemini-version" // IntegrityReactionsFeatureFlag enables reaction-based integrity promotion/demotion // in the MCPG allow-only policy. When enabled, the compiler injects // endorsement-reactions and disapproval-reactions fields into the allow-only policy. diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 02ad9051270..5ce6d4f1432 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -78,6 +78,7 @@ func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub string(constants.DefaultClaudeCodeVersion), "Install Claude Code CLI", "claude", + constants.ClaudeVersionFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/claude_engine_test.go b/pkg/workflow/claude_engine_test.go index 14d57db5d92..a224d7539ab 100644 --- a/pkg/workflow/claude_engine_test.go +++ b/pkg/workflow/claude_engine_test.go @@ -62,6 +62,16 @@ func TestClaudeEngine(t *testing.T) { t.Errorf("Expected '%s' in install step, got: %s", expectedInstallCommand, installStep) } + featureInstallSteps := engine.GetInstallationSteps(&WorkflowData{ + Features: map[string]any{ + string(constants.ClaudeVersionFeatureFlag): "latest", + }, + }) + featureInstallStep := strings.Join([]string(featureInstallSteps[1]), "\n") + if !strings.Contains(featureInstallStep, "npm install --ignore-scripts -g @anthropic-ai/claude-code@latest") { + t.Errorf("Expected claude-version feature override to use latest, got: %s", featureInstallStep) + } + // Test execution steps workflowData := &WorkflowData{ Name: "test-workflow", diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 049de526804..dd3ff85acb8 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -80,11 +80,17 @@ func (e *CodexEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA } // Use base installation steps (npm install only; secret validation is in the activation job) + codexVersion := resolveEngineInstallVersion( + string(constants.DefaultCodexVersion), + constants.CodexVersionFeatureFlag, + workflowData, + ) + steps := GetBaseInstallationSteps(EngineInstallConfig{ Secrets: []string{"CODEX_API_KEY", "OPENAI_API_KEY"}, DocsURL: "https://github.github.com/gh-aw/reference/engines/#openai-codex", NpmPackage: "@openai/codex", - Version: string(constants.DefaultCodexVersion), + Version: codexVersion, Name: "Codex CLI", InstallStepName: "Install Codex CLI", CliName: "codex", diff --git a/pkg/workflow/codex_engine_test.go b/pkg/workflow/codex_engine_test.go index d4680f3388c..09e1b52a202 100644 --- a/pkg/workflow/codex_engine_test.go +++ b/pkg/workflow/codex_engine_test.go @@ -135,6 +135,25 @@ func TestCodexEngineWithVersion(t *testing.T) { if !foundVersionInstall { t.Error("Expected versioned npm install command with @openai/codex@3.0.1") } + + // Test installation steps with feature-based version override + stepsWithFeatureVersion := engine.GetInstallationSteps(&WorkflowData{ + Features: map[string]any{ + string(constants.CodexVersionFeatureFlag): "latest", + }, + }) + foundFeatureVersionInstall := false + for _, step := range stepsWithFeatureVersion { + for _, line := range step { + if strings.Contains(line, "npm install") && strings.Contains(line, "@openai/codex@latest") { + foundFeatureVersionInstall = true + break + } + } + } + if !foundFeatureVersionInstall { + t.Error("Expected feature version override npm install command with @openai/codex@latest") + } } func TestCodexEngineExecutionIncludesGitHubAWPrompt(t *testing.T) { diff --git a/pkg/workflow/copilot_engine_installation.go b/pkg/workflow/copilot_engine_installation.go index 1b13a8ad4b1..cc01cd73dc4 100644 --- a/pkg/workflow/copilot_engine_installation.go +++ b/pkg/workflow/copilot_engine_installation.go @@ -65,10 +65,11 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu } // Determine Copilot version - copilotVersion := string(constants.DefaultCopilotVersion) - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { - copilotVersion = workflowData.EngineConfig.Version - } + copilotVersion := resolveEngineInstallVersion( + string(constants.DefaultCopilotVersion), + constants.CopilotVersionFeatureFlag, + workflowData, + ) if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { copilotVersion = "latest" copilotInstallLog.Print("byok-copilot enabled: forcing Copilot CLI install version to latest") diff --git a/pkg/workflow/copilot_installer_test.go b/pkg/workflow/copilot_installer_test.go index 561386d6194..75f1b4bebdd 100644 --- a/pkg/workflow/copilot_installer_test.go +++ b/pkg/workflow/copilot_installer_test.go @@ -281,3 +281,93 @@ func TestCopilotInstallerByokFeatureUsesLatestVersion(t *testing.T) { t.Errorf("Expected byok-copilot to ignore pinned version, got:\n%s", installStep) } } + +func TestCopilotInstallerCopilotVersionFeatureUsesLatestVersion(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var installStep string + for _, step := range steps { + stepContent := strings.Join(step, "\n") + if strings.Contains(stepContent, "install_copilot_cli.sh") { + installStep = stepContent + break + } + } + + if installStep == "" { + t.Fatal("Could not find install step with install_copilot_cli.sh") + } + + if !strings.Contains(installStep, `install_copilot_cli.sh" latest`) { + t.Errorf("Expected copilot-version=latest to force latest Copilot CLI install, got:\n%s", installStep) + } +} + +func TestCopilotInstallerEngineConfigVersionTakesPrecedenceOverFeatureVersion(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + Version: "1.0.0", + }, + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var installStep string + for _, step := range steps { + stepContent := strings.Join(step, "\n") + if strings.Contains(stepContent, "install_copilot_cli.sh") { + installStep = stepContent + break + } + } + + if installStep == "" { + t.Fatal("Could not find install step with install_copilot_cli.sh") + } + + if !strings.Contains(installStep, `install_copilot_cli.sh" 1.0.0`) { + t.Errorf("Expected engine.version to take precedence over copilot-version feature flag, got:\n%s", installStep) + } +} + +func TestCopilotInstallerCopilotVersionFeatureSupportsSpecificVersion(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "1.2.3", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var installStep string + for _, step := range steps { + stepContent := strings.Join(step, "\n") + if strings.Contains(stepContent, "install_copilot_cli.sh") { + installStep = stepContent + break + } + } + + if installStep == "" { + t.Fatal("Could not find install step with install_copilot_cli.sh") + } + + if !strings.Contains(installStep, `install_copilot_cli.sh" 1.2.3`) { + t.Errorf("Expected copilot-version to support specific version values, got:\n%s", installStep) + } +} diff --git a/pkg/workflow/crush_engine.go b/pkg/workflow/crush_engine.go index fdd630ca1c6..4e524c18c7f 100644 --- a/pkg/workflow/crush_engine.go +++ b/pkg/workflow/crush_engine.go @@ -103,6 +103,7 @@ func (e *CrushEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHubA string(constants.DefaultCrushVersion), "Install Crush CLI", "crush", + constants.NoFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/docker.go b/pkg/workflow/docker.go index 38bef5149de..dea5155ea38 100644 --- a/pkg/workflow/docker.go +++ b/pkg/workflow/docker.go @@ -125,12 +125,7 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio mcpGateway := workflowData.SandboxConfig.MCP if mcpGateway.Container != "" { image := mcpGateway.Container - if mcpGateway.Version != "" { - image += ":" + mcpGateway.Version - } else { - // Use default version if not specified (consistent with mcp_servers.go) - image += ":" + string(constants.DefaultMCPGatewayVersion) - } + image += ":" + getMCPGatewayVersion(workflowData, mcpGateway.Version) if !imageSet[image] { images = append(images, image) imageSet[image] = true diff --git a/pkg/workflow/docker_api_proxy_test.go b/pkg/workflow/docker_api_proxy_test.go index 3b3bf625f1c..1e73737cebe 100644 --- a/pkg/workflow/docker_api_proxy_test.go +++ b/pkg/workflow/docker_api_proxy_test.go @@ -79,3 +79,33 @@ func TestCollectDockerImages_APIProxyForEnginesWithLLMGateway(t *testing.T) { }) } } + +func TestCollectDockerImages_FirewallVersionFeatureOverride(t *testing.T) { + featureVersion := "v0.25.27" + featureTag := "0.25.27" + workflowData := &WorkflowData{ + AI: "claude", + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): featureVersion, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + images := collectDockerImages(nil, workflowData, ActionModeRelease) + + expectedImages := []string{ + constants.DefaultFirewallRegistry + "/squid:" + featureTag, + constants.DefaultFirewallRegistry + "/agent:" + featureTag, + constants.DefaultFirewallRegistry + "/api-proxy:" + featureTag, + } + + for _, expectedImage := range expectedImages { + if !slices.Contains(images, expectedImage) { + t.Errorf("Expected image %q in collected images, got %v", expectedImage, images) + } + } +} diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index b8d08739885..2610fa11d63 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -37,6 +37,7 @@ import ( "sort" "strings" + "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" ) @@ -107,6 +108,7 @@ func GetBaseInstallationSteps(config EngineInstallConfig, workflowData *Workflow config.Version, stepName, config.CliName, + constants.NoFeatureFlag, workflowData, ) steps = append(steps, npmSteps...) @@ -131,16 +133,12 @@ func BuildStandardNpmEngineInstallSteps( defaultVersion string, stepName string, cacheKeyPrefix string, + featureVersionFlag constants.FeatureFlag, workflowData *WorkflowData, ) []GitHubActionStep { engineHelpersLog.Printf("Building npm engine install steps: package=%s, version=%s", packageName, defaultVersion) - // Use version from engine config if provided, otherwise default to pinned version - version := defaultVersion - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { - version = workflowData.EngineConfig.Version - engineHelpersLog.Printf("Using engine config version: %s", version) - } + version := resolveEngineInstallVersion(defaultVersion, featureVersionFlag, workflowData) // Add npm package installation steps (includes Node.js setup) // Always pass false for runInstallScripts: engine CLI installs must never run @@ -156,6 +154,35 @@ func BuildStandardNpmEngineInstallSteps( ) } +// resolveEngineInstallVersion resolves engine CLI install version with this precedence: +// 1. explicit engine.version in frontmatter +// 2. feature flag version override (if provided) +// 3. default pinned version +// +// Note: Copilot currently applies an additional post-resolution override: +// byok-copilot forces "latest" after this resolver returns. Other engines +// should not add post-resolution overrides unless explicitly designed to. +func resolveEngineInstallVersion( + defaultVersion string, + featureVersionFlag constants.FeatureFlag, + workflowData *WorkflowData, +) string { + hasExplicitEngineVersion := workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" + if hasExplicitEngineVersion { + engineHelpersLog.Printf("Using engine config version: %s", workflowData.EngineConfig.Version) + return workflowData.EngineConfig.Version + } + + if featureVersionFlag != constants.NoFeatureFlag { + if featureVersion := getFeatureValue(featureVersionFlag, workflowData); featureVersion != "" { + engineHelpersLog.Printf("Using feature override version (%s): %s", featureVersionFlag, featureVersion) + return featureVersion + } + } + + return defaultVersion +} + // BuildNpmEngineInstallStepsWithAWF injects an AWF installation step between the Node.js // setup step and the CLI install steps when the firewall is enabled. This eliminates the // duplicated AWF-injection pattern shared by Claude, Gemini, and Copilot engines. diff --git a/pkg/workflow/engine_helpers_test.go b/pkg/workflow/engine_helpers_test.go index e72f73ebae7..8bb5fec79e1 100644 --- a/pkg/workflow/engine_helpers_test.go +++ b/pkg/workflow/engine_helpers_test.go @@ -18,12 +18,15 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { tests := []struct { name string workflowData *WorkflowData + featureFlag constants.FeatureFlag expectedSteps int // Number of steps expected (Node.js setup + npm install) expectedInStep string + unexpectedInStep string }{ { name: "with default version", workflowData: &WorkflowData{}, + featureFlag: constants.NoFeatureFlag, expectedSteps: 2, // Node.js setup + npm install expectedInStep: string(constants.DefaultCopilotVersion), }, @@ -34,6 +37,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { Version: "1.2.3", }, }, + featureFlag: constants.CopilotVersionFeatureFlag, expectedSteps: 2, expectedInStep: "1.2.3", }, @@ -44,9 +48,36 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { Version: "", }, }, + featureFlag: constants.NoFeatureFlag, expectedSteps: 2, expectedInStep: string(constants.DefaultCopilotVersion), }, + { + name: "with version override from feature flag", + workflowData: &WorkflowData{ + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + }, + featureFlag: constants.CopilotVersionFeatureFlag, + expectedSteps: 2, + expectedInStep: "latest", + }, + { + name: "engine config version takes precedence over feature override", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + Version: "1.2.3", + }, + Features: map[string]any{ + string(constants.CopilotVersionFeatureFlag): "latest", + }, + }, + featureFlag: constants.CopilotVersionFeatureFlag, + expectedSteps: 2, + expectedInStep: "1.2.3", + unexpectedInStep: "latest", + }, } for _, tt := range tests { @@ -56,6 +87,7 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { string(constants.DefaultCopilotVersion), "Install GitHub Copilot CLI", "copilot", + tt.featureFlag, tt.workflowData, ) @@ -77,6 +109,16 @@ func TestBuildStandardNpmEngineInstallSteps(t *testing.T) { if !found { t.Errorf("Expected version %s not found in steps", tt.expectedInStep) } + + if tt.unexpectedInStep != "" { + for _, step := range steps { + for _, line := range step { + if strings.Contains(line, tt.unexpectedInStep) { + t.Errorf("Did not expect to find %q in steps when explicit version takes precedence", tt.unexpectedInStep) + } + } + } + } }) } } @@ -121,6 +163,7 @@ func TestBuildStandardNpmEngineInstallSteps_AllEngines(t *testing.T) { tt.defaultVersion, tt.stepName, tt.cacheKeyPrefix, + constants.NoFeatureFlag, workflowData, ) diff --git a/pkg/workflow/features.go b/pkg/workflow/features.go index 2c7a0126922..a122d26f13c 100644 --- a/pkg/workflow/features.go +++ b/pkg/workflow/features.go @@ -86,3 +86,44 @@ func isFeatureEnabled(flag constants.FeatureFlag, workflowData *WorkflowData) bo featuresLog.Printf("Feature not found: %s=false", flagLower) return false } + +// getFeatureValue returns the string value for a feature flag from workflow frontmatter. +// Returns an empty string when: +// - workflowData is nil +// - features are not configured +// - the feature is missing +// - the feature value is not a string +// - the feature value is an empty/whitespace string +func getFeatureValue(flag constants.FeatureFlag, workflowData *WorkflowData) string { + if workflowData == nil || workflowData.Features == nil { + return "" + } + + flagLower := strings.ToLower(strings.TrimSpace(string(flag))) + + if value, exists := workflowData.Features[flagLower]; exists { + if strVal, ok := value.(string); ok { + trimmed := strings.TrimSpace(strVal) + if trimmed != "" { + featuresLog.Printf("Feature value found in frontmatter: %s=%s", flagLower, trimmed) + } + return trimmed + } + return "" + } + + for key, value := range workflowData.Features { + if strings.ToLower(key) == flagLower { + if strVal, ok := value.(string); ok { + trimmed := strings.TrimSpace(strVal) + if trimmed != "" { + featuresLog.Printf("Feature value found in frontmatter (case-insensitive): %s=%s", flagLower, trimmed) + } + return trimmed + } + return "" + } + } + + return "" +} diff --git a/pkg/workflow/features_test.go b/pkg/workflow/features_test.go index f4c9fb02ca6..fadc694b18d 100644 --- a/pkg/workflow/features_test.go +++ b/pkg/workflow/features_test.go @@ -302,3 +302,78 @@ func TestMergedFeaturesTopLevelPrecedence(t *testing.T) { t.Errorf("isFeatureEnabled(\"import-only\") = %v, want true (from import)", importOnlyResult) } } + +func TestGetFeatureValue(t *testing.T) { + tests := []struct { + name string + workflow *WorkflowData + flag constants.FeatureFlag + expectedVal string + }{ + { + name: "returns exact string value", + workflow: &WorkflowData{ + Features: map[string]any{ + "mcpg-version": "v0.2.27", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "v0.2.27", + }, + { + name: "returns case-insensitive key match", + workflow: &WorkflowData{ + Features: map[string]any{ + "MCPG-VERSION": "v0.2.28", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "v0.2.28", + }, + { + name: "trims whitespace from value", + workflow: &WorkflowData{ + Features: map[string]any{ + "mcpg-version": " v0.2.29 ", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "v0.2.29", + }, + { + name: "returns empty for non-string value", + workflow: &WorkflowData{ + Features: map[string]any{ + "mcpg-version": true, + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "", + }, + { + name: "returns empty for missing value", + workflow: &WorkflowData{ + Features: map[string]any{ + "other-feature": "v1", + }, + }, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "", + }, + { + name: "returns empty for nil workflow", + workflow: nil, + flag: constants.MCPGVersionFeatureFlag, + expectedVal: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getFeatureValue(tt.flag, tt.workflow) + if got != tt.expectedVal { + t.Errorf("getFeatureValue(%q) = %q, want %q", tt.flag, got, tt.expectedVal) + } + }) + } +} diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index a04060a36d5..b8cff744b9c 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -65,9 +65,20 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return nil } + featureVersion := getFeatureValue(constants.FirewallVersionFeatureFlag, workflowData) + // Check network.firewall configuration if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil { config := workflowData.NetworkPermissions.Firewall + if config.Version == "" && featureVersion != "" { + // Use a copy to avoid mutating parsed frontmatter structures. + configCopy := *config + configCopy.Version = featureVersion + if firewallLog.Enabled() { + firewallLog.Printf("Applied firewall-version feature override: version=%s", featureVersion) + } + return &configCopy + } if firewallLog.Enabled() { firewallLog.Printf("Retrieved firewall config: enabled=%v, version=%s, logLevel=%s", config.Enabled, config.Version, config.LogLevel) @@ -75,6 +86,13 @@ func getFirewallConfig(workflowData *WorkflowData) *FirewallConfig { return config } + if featureVersion != "" { + if firewallLog.Enabled() { + firewallLog.Printf("Using firewall-version feature override without explicit firewall config: version=%s", featureVersion) + } + return &FirewallConfig{Version: featureVersion} + } + return nil } diff --git a/pkg/workflow/firewall_image_tag_test.go b/pkg/workflow/firewall_image_tag_test.go index c85bc1189ba..5109ce91047 100644 --- a/pkg/workflow/firewall_image_tag_test.go +++ b/pkg/workflow/firewall_image_tag_test.go @@ -119,6 +119,37 @@ func TestClaudeEngineAWFImageTag(t *testing.T) { t.Errorf("Expected AWF command to contain '%s', got:\n%s", expectedImageTag, stepContent) } }) + + t.Run("AWF command includes image-tag with firewall-version feature override", func(t *testing.T) { + featureVersion := "v0.25.27" + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "claude", + }, + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): featureVersion, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + engine := NewClaudeEngine() + steps := engine.GetExecutionSteps(workflowData, "test.log") + + if len(steps) == 0 { + t.Fatal("Expected at least one execution step") + } + + stepContent := strings.Join(steps[0], "\n") + expectedImageTag := "--image-tag " + strings.TrimPrefix(featureVersion, "v") + if !strings.Contains(stepContent, expectedImageTag) { + t.Errorf("Expected AWF command to contain '%s', got:\n%s", expectedImageTag, stepContent) + } + }) } // TestCodexEngineAWFImageTag tests that Codex engine includes --image-tag in AWF commands diff --git a/pkg/workflow/firewall_version_pinning_test.go b/pkg/workflow/firewall_version_pinning_test.go index dacb807be51..90893f5a778 100644 --- a/pkg/workflow/firewall_version_pinning_test.go +++ b/pkg/workflow/firewall_version_pinning_test.go @@ -158,6 +158,83 @@ func TestCopilotEngineFirewallInstallation(t *testing.T) { } }) + t.Run("uses feature override version when firewall config version is not specified", func(t *testing.T) { + engine := NewCopilotEngine() + featureVersion := "v0.25.27" + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): featureVersion, + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var foundAWFStep bool + var awfStepStr string + for _, step := range steps { + stepStr := strings.Join(step, "\n") + if strings.Contains(stepStr, "Install AWF binary") { + foundAWFStep = true + awfStepStr = stepStr + break + } + } + + if !foundAWFStep { + t.Fatal("Expected to find AWF installation step when firewall is enabled") + } + + if !strings.Contains(awfStepStr, featureVersion) { + t.Errorf("AWF installation step should pass feature override version %s to script", featureVersion) + } + }) + + t.Run("supports latest feature override version", func(t *testing.T) { + engine := NewCopilotEngine() + workflowData := &WorkflowData{ + Name: "test-workflow", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + Features: map[string]any{ + string(constants.FirewallVersionFeatureFlag): "latest", + }, + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + }, + }, + } + + steps := engine.GetInstallationSteps(workflowData) + + var awfStepStr string + for _, step := range steps { + stepStr := strings.Join(step, "\n") + if strings.Contains(stepStr, "Install AWF binary") { + awfStepStr = stepStr + break + } + } + + if awfStepStr == "" { + t.Fatal("Expected to find AWF installation step when firewall is enabled") + } + + if !strings.Contains(awfStepStr, `install_awf_binary.sh" latest`) { + t.Errorf("AWF installation step should pass latest to script when firewall-version=latest, got:\n%s", awfStepStr) + } + }) + t.Run("does not include AWF installation when firewall disabled", func(t *testing.T) { engine := NewCopilotEngine() workflowData := &WorkflowData{ diff --git a/pkg/workflow/gemini_engine.go b/pkg/workflow/gemini_engine.go index 05d9d5b676b..2324c77edf2 100644 --- a/pkg/workflow/gemini_engine.go +++ b/pkg/workflow/gemini_engine.go @@ -91,6 +91,7 @@ func (e *GeminiEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub string(constants.DefaultGeminiVersion), "Install Gemini CLI", "gemini", + constants.GeminiVersionFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/gemini_engine_test.go b/pkg/workflow/gemini_engine_test.go index 454722b2223..3653a1e891c 100644 --- a/pkg/workflow/gemini_engine_test.go +++ b/pkg/workflow/gemini_engine_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/github/gh-aw/pkg/constants" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -111,6 +112,20 @@ func TestGeminiEngineInstallation(t *testing.T) { assert.Empty(t, steps, "Should skip installation when custom command is specified") }) + t.Run("feature version override supports latest", func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.GeminiVersionFeatureFlag): "latest", + }, + } + + steps := engine.GetInstallationSteps(workflowData) + require.GreaterOrEqual(t, len(steps), 2, "Should have install step") + stepContent := strings.Join(steps[1], "\n") + assert.Contains(t, stepContent, "@google/gemini-cli@latest", "Should install latest Gemini CLI when gemini-version is latest") + }) + t.Run("with firewall", func(t *testing.T) { workflowData := &WorkflowData{ Name: "test-workflow", diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index f191b24a6a1..90a4f6f79da 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -52,6 +52,16 @@ import ( var mcpGatewayConfigLog = logger.New("workflow:mcp_gateway_config") +func getMCPGatewayVersion(workflowData *WorkflowData, configuredVersion string) string { + if configuredVersion != "" { + return configuredVersion + } + if featureVersion := getFeatureValue(constants.MCPGVersionFeatureFlag, workflowData); featureVersion != "" { + return featureVersion + } + return string(constants.DefaultMCPGatewayVersion) +} + // ensureDefaultMCPGatewayConfig ensures MCP gateway has default configuration if not provided // The MCP gateway is mandatory and defaults to github/gh-aw-mcpg func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) { @@ -69,7 +79,7 @@ func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) { mcpGatewayConfigLog.Print("No MCP gateway configuration found, setting default configuration") workflowData.SandboxConfig.MCP = &MCPGatewayRuntimeConfig{ Container: constants.DefaultMCPGatewayContainer, - Version: string(constants.DefaultMCPGatewayVersion), + Version: getMCPGatewayVersion(workflowData, ""), Port: int(DefaultMCPGatewayPort), } } else { @@ -77,9 +87,9 @@ func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) { if workflowData.SandboxConfig.MCP.Container == "" { workflowData.SandboxConfig.MCP.Container = constants.DefaultMCPGatewayContainer } - // Only replace empty version with default - preserve user-specified versions including "latest" + // Only replace empty version - preserve user-specified versions including "latest" if workflowData.SandboxConfig.MCP.Version == "" { - workflowData.SandboxConfig.MCP.Version = string(constants.DefaultMCPGatewayVersion) + workflowData.SandboxConfig.MCP.Version = getMCPGatewayVersion(workflowData, "") } if workflowData.SandboxConfig.MCP.Port == 0 { workflowData.SandboxConfig.MCP.Port = int(DefaultMCPGatewayPort) diff --git a/pkg/workflow/mcp_gateway_config_test.go b/pkg/workflow/mcp_gateway_config_test.go index 840796557ca..94510f0d3e0 100644 --- a/pkg/workflow/mcp_gateway_config_test.go +++ b/pkg/workflow/mcp_gateway_config_test.go @@ -68,6 +68,45 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) { assert.Equal(t, 8080, wd.SandboxConfig.MCP.Port, "Port should be preserved") }, }, + { + name: "fills in missing version field from mcpg-version feature override", + workflowData: &WorkflowData{ + Features: map[string]any{ + string(constants.MCPGVersionFeatureFlag): "v0.2.27", + }, + SandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{ + Container: "custom-container", + Port: 8080, + }, + }, + }, + validate: func(t *testing.T, wd *WorkflowData) { + assert.Equal(t, "custom-container", wd.SandboxConfig.MCP.Container, "Container should be preserved") + assert.Equal(t, "v0.2.27", wd.SandboxConfig.MCP.Version, "Version should use feature override") + assert.Equal(t, 8080, wd.SandboxConfig.MCP.Port, "Port should be preserved") + }, + }, + { + name: "preserves explicit version over mcpg-version feature override", + workflowData: &WorkflowData{ + Features: map[string]any{ + string(constants.MCPGVersionFeatureFlag): "v0.2.27", + }, + SandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{ + Container: "custom-container", + Version: "v0.3.0", + Port: 8080, + }, + }, + }, + validate: func(t *testing.T, wd *WorkflowData) { + assert.Equal(t, "custom-container", wd.SandboxConfig.MCP.Container, "Container should be preserved") + assert.Equal(t, "v0.3.0", wd.SandboxConfig.MCP.Version, "Explicit version should be preserved") + assert.Equal(t, 8080, wd.SandboxConfig.MCP.Port, "Port should be preserved") + }, + }, { name: "fills in missing port field", workflowData: &WorkflowData{ diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index b75eee3b1bd..4f9e25c8204 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -688,11 +688,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, // Build container command containerImage := gatewayConfig.Container - if gatewayConfig.Version != "" { - containerImage += ":" + gatewayConfig.Version - } else { - containerImage += ":" + string(constants.DefaultMCPGatewayVersion) - } + containerImage += ":" + getMCPGatewayVersion(workflowData, gatewayConfig.Version) var containerCmd strings.Builder containerCmd.WriteString("docker run -i --rm --network host") diff --git a/pkg/workflow/mcp_setup_generator_test.go b/pkg/workflow/mcp_setup_generator_test.go index bda8e6cc2bd..8910ac1c15b 100644 --- a/pkg/workflow/mcp_setup_generator_test.go +++ b/pkg/workflow/mcp_setup_generator_test.go @@ -118,6 +118,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { tests := []struct { name string sandboxConfig *SandboxConfig + features map[string]any expectedVersion string description string }{ @@ -180,6 +181,20 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { expectedVersion: "1.2.3", description: "should use custom version 1.2.3", }, + { + name: "mcpg-version feature override applies when sandbox.mcp.version is not set", + sandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{ + Container: constants.DefaultMCPGatewayContainer, + Port: 8080, + }, + }, + features: map[string]any{ + string(constants.MCPGVersionFeatureFlag): "v0.2.27", + }, + expectedVersion: "v0.2.27", + description: "should use mcpg-version feature override when no explicit sandbox.mcp.version is set", + }, } for _, tt := range tests { @@ -187,6 +202,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) { workflowData := &WorkflowData{ SandboxConfig: tt.sandboxConfig, Tools: map[string]any{"github": map[string]any{}}, + Features: tt.features, } // Ensure MCP gateway config is applied (includes normalization of "latest") diff --git a/pkg/workflow/opencode_engine.go b/pkg/workflow/opencode_engine.go index 879b59cd892..ee3beda00e0 100644 --- a/pkg/workflow/opencode_engine.go +++ b/pkg/workflow/opencode_engine.go @@ -96,6 +96,7 @@ func (e *OpenCodeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitH string(constants.DefaultOpenCodeVersion), "Install OpenCode CLI", "opencode", + constants.OpenCodeVersionFeatureFlag, workflowData, ) return BuildNpmEngineInstallStepsWithAWF(npmSteps, workflowData) diff --git a/pkg/workflow/opencode_engine_test.go b/pkg/workflow/opencode_engine_test.go index 4938b21d074..1004b881012 100644 --- a/pkg/workflow/opencode_engine_test.go +++ b/pkg/workflow/opencode_engine_test.go @@ -39,6 +39,18 @@ func TestOpenCodeEngineInstallationAndExecution(t *testing.T) { assert.Contains(t, stepContent, "Setup Node.js", "Should include Node setup") }) + t.Run("feature version override supports latest", func(t *testing.T) { + steps := engine.GetInstallationSteps(&WorkflowData{ + Name: "test-workflow", + Features: map[string]any{ + string(constants.OpenCodeVersionFeatureFlag): "latest", + }, + }) + require.GreaterOrEqual(t, len(steps), 2, "Should generate install step") + stepContent := strings.Join(steps[1], "\n") + assert.Contains(t, stepContent, "opencode-ai@latest", "Should install latest OpenCode CLI when opencode-version is latest") + }) + t.Run("execution uses opencode command and config", func(t *testing.T) { steps := engine.GetExecutionSteps(&WorkflowData{Name: "test-workflow"}, "/tmp/test.log") require.Len(t, steps, 2, "Should generate config step and execution step")