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
11 changes: 11 additions & 0 deletions .github/aw/test-expression.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
on:
workflow_call:
inputs:
engine-version:
type: string
engine:
id: copilot
version: ${{ inputs.engine-version }}
---
Fix the bug
4 changes: 2 additions & 2 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -9026,8 +9026,8 @@
},
"version": {
"type": ["string", "number"],
"description": "Optional version of the AI engine action (e.g., 'beta', 'stable', 20). Has sensible defaults and can typically be omitted. Numeric values are automatically converted to strings at runtime.",
"examples": ["beta", "stable", 20, 3.11]
"description": "Optional version of the AI engine action (e.g., 'beta', 'stable', 20). Has sensible defaults and can typically be omitted. Numeric values are automatically converted to strings at runtime. GitHub Actions expressions (e.g., '${{ inputs.engine-version }}') are accepted and compiled with injection-safe env var handling.",
"examples": ["beta", "stable", 20, 3.11, "${{ inputs.engine-version }}"]
},
"model": {
"type": "string",
Expand Down
37 changes: 37 additions & 0 deletions pkg/workflow/claude_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,3 +513,40 @@ func TestClaudeEngineEnvOverridesTokenExpression(t *testing.T) {
}
})
}

func TestClaudeEngineWithExpressionVersion(t *testing.T) {
engine := NewClaudeEngine()

expressionVersion := "${{ inputs.engine-version }}"
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "claude",
Version: expressionVersion,
},
}

// Expression version must use env var injection in the install step
installSteps := engine.GetInstallationSteps(workflowData)
// Expect: Node.js setup step + install step
if len(installSteps) != 2 {
t.Fatalf("Expected 2 installation steps, got %d", len(installSteps))
}

installStep := strings.Join([]string(installSteps[1]), "\n")

// Should use ENGINE_VERSION env var
if !strings.Contains(installStep, "ENGINE_VERSION: "+expressionVersion) {
t.Errorf("Expected ENGINE_VERSION env var in install step, got:\n%s", installStep)
}

// Should reference env var in command
if !strings.Contains(installStep, `"${ENGINE_VERSION}"`) {
t.Errorf(`Expected "$ENGINE_VERSION" in npm install command, got:\n%s`, installStep)
}

// Should NOT embed expression directly in shell command
if strings.Contains(installStep, "@anthropic-ai/claude-code@"+expressionVersion) {
t.Errorf("Expression should NOT be embedded directly in npm install command, got:\n%s", installStep)
}
}
44 changes: 44 additions & 0 deletions pkg/workflow/codex_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -771,3 +771,47 @@ func TestCodexEngineWebSearch(t *testing.T) {
}
})
}

func TestCodexEngineWithExpressionVersion(t *testing.T) {
engine := NewCodexEngine()

expressionVersion := "${{ inputs.engine-version }}"
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "codex",
Version: expressionVersion,
},
}

installSteps := engine.GetInstallationSteps(workflowData)

// Find the npm install step
var installStep string
for _, step := range installSteps {
stepContent := strings.Join(step, "\n")
if strings.Contains(stepContent, "npm install") {
installStep = stepContent
break
}
}

if installStep == "" {
t.Fatal("Could not find npm install step")
}

// Should use ENGINE_VERSION env var for injection safety
if !strings.Contains(installStep, "ENGINE_VERSION: "+expressionVersion) {
t.Errorf("Expected ENGINE_VERSION env var in install step, got:\n%s", installStep)
}

// Should reference env var in command
if !strings.Contains(installStep, `"${ENGINE_VERSION}"`) {
t.Errorf(`Expected "$ENGINE_VERSION" in npm install command, got:\n%s`, installStep)
}

// Should NOT embed expression directly in npm install command
if strings.Contains(installStep, "@openai/codex@"+expressionVersion) {
t.Errorf("Expression should NOT be embedded directly in npm install command, got:\n%s", installStep)
}
}
13 changes: 13 additions & 0 deletions pkg/workflow/copilot_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ func GenerateCopilotInstallerSteps(version, stepName string) []GitHubActionStep
// and does not use gh CLI, so GH_HOST does not affect the download. No step-level
// GH_HOST override is needed here; the correct host is already set in GITHUB_ENV
// by configure_gh_for_ghe.sh (or by the Derive GH_HOST step when DIFC proxy is active).
if ExpressionPattern.MatchString(version) {
// Version is a GitHub Actions expression (e.g. ${{ inputs.engine-version }}).
// Pass it via an env var instead of direct shell interpolation to prevent injection.
copilotInstallerLog.Printf("Version contains GitHub Actions expression, using env var for injection safety: %s", version)
stepLines := []string{
" - name: " + stepName,
` run: ${RUNNER_TEMP}/gh-aw/actions/install_copilot_cli.sh "${ENGINE_VERSION}"`,
" env:",
" ENGINE_VERSION: " + version,
}
return []GitHubActionStep{GitHubActionStep(stepLines)}
}

stepLines := []string{
" - name: " + stepName,
" run: ${RUNNER_TEMP}/gh-aw/actions/install_copilot_cli.sh " + version,
Expand Down
96 changes: 96 additions & 0 deletions pkg/workflow/copilot_installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,99 @@ func TestCopilotInstallerCustomVersion(t *testing.T) {
t.Errorf("Install step should NOT hardcode GH_HOST: github.com (breaks GHEC), got:\n%s", installStep)
}
}

func TestGenerateCopilotInstallerSteps_ExpressionVersion(t *testing.T) {
tests := []struct {
name string
version string
envVar string
}{
{
name: "workflow_call input expression",
version: "${{ inputs.engine-version }}",
envVar: "ENGINE_VERSION: ${{ inputs.engine-version }}",
},
{
name: "github event input expression",
version: "${{ github.event.inputs.engine-version }}",
envVar: "ENGINE_VERSION: ${{ github.event.inputs.engine-version }}",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
steps := GenerateCopilotInstallerSteps(tt.version, "Install GitHub Copilot CLI")

if len(steps) != 1 {
t.Errorf("Expected 1 step, got %d", len(steps))
return
}

stepContent := strings.Join(steps[0], "\n")

// Should use env var section
if !strings.Contains(stepContent, "env:") {
t.Errorf("Expected step to contain 'env:' section for expression version, got:\n%s", stepContent)
}

// Should define ENGINE_VERSION env var with the expression
if !strings.Contains(stepContent, tt.envVar) {
t.Errorf("Expected step to contain %q, got:\n%s", tt.envVar, stepContent)
}

// Should reference ENGINE_VERSION in the run command
if !strings.Contains(stepContent, `"${ENGINE_VERSION}"`) {
t.Errorf(`Expected step to use "$ENGINE_VERSION" in run command, got:\n%s`, stepContent)
}

// Should NOT embed the expression directly in the shell command
if strings.Contains(stepContent, "install_copilot_cli.sh "+tt.version) {
t.Errorf("Expression version should NOT be embedded directly in shell command, got:\n%s", stepContent)
}
})
}
}

func TestCopilotInstallerExpressionVersion_ViaEngineConfig(t *testing.T) {
// Test that expression version from engine config uses env var injection
engine := NewCopilotEngine()

expressionVersion := "${{ inputs.engine-version }}"
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Version: expressionVersion,
},
}

steps := engine.GetInstallationSteps(workflowData)

// Find the install step
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")
}

// Should use env var for injection safety
if !strings.Contains(installStep, "ENGINE_VERSION: ${{ inputs.engine-version }}") {
t.Errorf("Expected ENGINE_VERSION env var in install step, got:\n%s", installStep)
}

// Should reference env var in run command
if !strings.Contains(installStep, `"${ENGINE_VERSION}"`) {
t.Errorf(`Expected "$ENGINE_VERSION" in run command, got:\n%s`, installStep)
}

// Should NOT embed expression directly in shell command
if strings.Contains(installStep, "install_copilot_cli.sh "+expressionVersion) {
t.Errorf("Expression should NOT be embedded directly in shell command, got:\n%s", installStep)
}
}
11 changes: 11 additions & 0 deletions pkg/workflow/engine_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ func TestExtractEngineConfig(t *testing.T) {
expectedEngineSetting: "claude",
expectedConfig: &EngineConfig{ID: "claude", Version: "beta"},
},
{
name: "object format - with expression version",
frontmatter: map[string]any{
"engine": map[string]any{
"id": "copilot",
"version": "${{ inputs.engine-version }}",
},
},
expectedEngineSetting: "copilot",
expectedConfig: &EngineConfig{ID: "copilot", Version: "${{ inputs.engine-version }}"},
},
{
name: "object format - with integer version",
frontmatter: map[string]any{
Expand Down
44 changes: 44 additions & 0 deletions pkg/workflow/gemini_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,3 +526,47 @@ func TestGenerateGeminiSettingsStep(t *testing.T) {
assert.Contains(t, content, "GH_AW_GEMINI_BASE_CONFIG: '", "JSON env var value must be single-quoted for valid YAML")
})
}

func TestGeminiEngineWithExpressionVersion(t *testing.T) {
engine := NewGeminiEngine()

expressionVersion := "${{ inputs.engine-version }}"
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "gemini",
Version: expressionVersion,
},
}

installSteps := engine.GetInstallationSteps(workflowData)

// Find the npm install step
var installStep string
for _, step := range installSteps {
stepContent := strings.Join([]string(step), "\n")
if strings.Contains(stepContent, "npm install") {
installStep = stepContent
break
}
}

if installStep == "" {
t.Fatal("Could not find npm install step")
}

// Should use ENGINE_VERSION env var for injection safety
if !strings.Contains(installStep, "ENGINE_VERSION: "+expressionVersion) {
t.Errorf("Expected ENGINE_VERSION env var in install step, got:\n%s", installStep)
}

// Should reference env var in command
if !strings.Contains(installStep, `"${ENGINE_VERSION}"`) {
t.Errorf(`Expected "$ENGINE_VERSION" in npm install command, got:\n%s`, installStep)
}

// Should NOT embed expression directly in npm install command
if strings.Contains(installStep, "@google/gemini-cli@"+expressionVersion) {
t.Errorf("Expression should NOT be embedded directly in npm install command, got:\n%s", installStep)
}
}
28 changes: 23 additions & 5 deletions pkg/workflow/nodejs.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,29 @@ func GenerateNpmInstallStepsWithScope(packageName, version, stepName, cacheKeyPr
if isGlobal {
globalFlag = "-g "
}
installCmd := fmt.Sprintf("npm install %s%s@%s", globalFlag, packageName, version)
steps = append(steps, GitHubActionStep{
" - name: " + stepName,
" run: " + installCmd,
})

var installStep GitHubActionStep
if ExpressionPattern.MatchString(version) {
// Version is a GitHub Actions expression (e.g. ${{ inputs.engine-version }}).
// Pass it via an env var instead of direct shell interpolation to prevent injection:
// if the expression evaluates to a malicious string, it would otherwise be
// substituted verbatim into the shell command before the shell parses it.
nodejsLog.Printf("Version contains GitHub Actions expression, using env var for injection safety: %s", version)
installCmd := fmt.Sprintf(`npm install %s%s@"${ENGINE_VERSION}"`, globalFlag, packageName)
installStep = GitHubActionStep{
" - name: " + stepName,
" run: " + installCmd,
Comment on lines +62 to +66
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The expression-handling branch builds installCmd using a Go raw string that includes \"${ENGINE_VERSION}\" (backslash-escaped quotes). That will emit backslashes into the YAML, so the shell treats the quotes as literal characters and ${ENGINE_VERSION} expands unquoted—defeating the intended injection-safety. Remove the backslashes so the final shell command actually double-quotes the env var expansion.

Copilot uses AI. Check for mistakes.
" env:",
" ENGINE_VERSION: " + version,
}
} else {
installCmd := fmt.Sprintf("npm install %s%s@%s", globalFlag, packageName, version)
installStep = GitHubActionStep{
" - name: " + stepName,
" run: " + installCmd,
}
}
steps = append(steps, installStep)

return steps
}
Loading