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
48 changes: 13 additions & 35 deletions pkg/workflow/codex_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func NewCodexEngine() *CodexEngine {
supportsMaxTurns: false, // Codex does not support max-turns feature
supportsMaxContinuations: false, // Codex does not support --max-autopilot-continues-style continuation mode
supportsWebSearch: true, // Codex has built-in web-search support
supportsNativeAgentFile: true, // Codex reads the agent file and prepends it to the prompt at runtime
supportsNativeAgentFile: false, // Codex does not support agent file natively; the compiler prepends the agent file content to prompt.txt
llmGatewayPort: constants.CodexLLMGatewayPort,
},
}
Expand Down Expand Up @@ -136,8 +136,8 @@ func (e *CodexEngine) GetAgentManifestPathPrefixes() []string {
func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile string) []GitHubActionStep {
modelConfigured := workflowData.EngineConfig != nil && workflowData.EngineConfig.Model != ""
firewallEnabled := isFirewallEnabled(workflowData)
codexEngineLog.Printf("Building Codex execution steps: workflow=%s, modelConfigured=%v, has_agent_file=%v, firewall=%v",
workflowData.Name, modelConfigured, workflowData.AgentFile != "", firewallEnabled)
codexEngineLog.Printf("Building Codex execution steps: workflow=%s, modelConfigured=%v, firewall=%v",
workflowData.Name, modelConfigured, firewallEnabled)

var steps []GitHubActionStep

Expand Down Expand Up @@ -224,20 +224,11 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri
// However, npm-installed CLIs (like codex) need hostedtoolcache bin directories in PATH.
npmPathSetup := GetNpmBinPathSetup()

// Codex reads both agent file and prompt inside AWF container (PATH setup + agent file reading + codex command)
var codexCommandWithSetup string
if workflowData.AgentFile != "" {
agentPath, err := ResolveAgentFilePath(workflowData.AgentFile)
if err != nil {
codexEngineLog.Printf("Error resolving agent file path: %v", err)
return BuildInvalidAgentPathStep("Execute Codex CLI", workflowData.AgentFile, err)
}
// Read agent file and prompt inside AWF container, with PATH setup for npm binaries
codexCommandWithSetup = fmt.Sprintf(`%s && AGENT_CONTENT="$(awk 'BEGIN{skip=1} /^---$/{if(skip){skip=0;next}else{skip=1;next}} !skip' %s)" && INSTRUCTION="$(printf "%%s\n\n%%s" "$AGENT_CONTENT" "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)")" && %s`, npmPathSetup, agentPath, codexCommand)
} else {
// Read prompt inside AWF container to avoid Docker Compose interpolation issues, with PATH setup
codexCommandWithSetup = fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand)
}
// Codex reads prompt inside AWF container (PATH setup + codex command).
// For engines that do not support native agent-file handling (including Codex),
// the compiler prepends the agent file content to prompt.txt so no special
// shell variable juggling is needed here.
codexCommandWithSetup := fmt.Sprintf(`%s && INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" && %s`, npmPathSetup, codexCommand)

command = BuildAWFCommand(AWFCommandConfig{
EngineName: "codex",
Expand All @@ -255,29 +246,16 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri
ExcludeEnvVarNames: ComputeAWFExcludeEnvVarNames(workflowData, []string{"CODEX_API_KEY", "OPENAI_API_KEY"}),
})
} else {
// Build the command without AWF wrapping
// Reuse commandName already determined above
if workflowData.AgentFile != "" {
agentPath, err := ResolveAgentFilePath(workflowData.AgentFile)
if err != nil {
codexEngineLog.Printf("Error resolving agent file path: %v", err)
return BuildInvalidAgentPathStep("Execute Codex CLI", workflowData.AgentFile, err)
}
command = fmt.Sprintf(`set -o pipefail
touch %s
(umask 177 && touch %s)
AGENT_CONTENT="$(awk 'BEGIN{skip=1} /^---$/{if(skip){skip=0;next}else{skip=1;next}} !skip' %s)"
INSTRUCTION="$(printf "%%s\n\n%%s" "$AGENT_CONTENT" "$(cat "$GH_AW_PROMPT")")"
mkdir -p "$CODEX_HOME/logs"
%s %sexec%s%s%s%s"$INSTRUCTION" 2>&1 | tee %s`, AgentStepSummaryPath, logFile, agentPath, commandName, modelParam, webSearchParam, webFetchParam, fullAutoParam, customArgsParam, logFile)
} else {
command = fmt.Sprintf(`set -o pipefail
// Build the command without AWF wrapping.
// For engines that do not support native agent-file handling (including Codex),
// the compiler prepends the agent file content to prompt.txt so no special
// shell variable juggling is needed here.
command = fmt.Sprintf(`set -o pipefail
touch %s
(umask 177 && touch %s)
INSTRUCTION="$(cat "$GH_AW_PROMPT")"
mkdir -p "$CODEX_HOME/logs"
%s %sexec%s%s%s%s"$INSTRUCTION" 2>&1 | tee %s`, AgentStepSummaryPath, logFile, commandName, modelParam, webSearchParam, webFetchParam, fullAutoParam, customArgsParam, logFile)
}
}

// Get effective GitHub token based on precedence: custom token > default
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/compiler_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, pre
// - Agent files WITHOUT inputs: path is in data.ImportPaths → included by Step 1b.
// - Agent files WITH inputs: content is in data.ImportedMarkdown → included by Step 1a.
// - inlined-imports mode: data.AgentFile is cleared; content is in data.ImportPaths.
// Engines that DO support native agent-file handling (e.g. Codex) continue to read the
// file themselves in GetExecutionSteps alongside the runtime-import.
// All current engines (Claude, Codex, Gemini, Copilot) use this mechanism: SupportsNativeAgentFile()
// returns false and they read the fully-assembled prompt.txt in GetExecutionSteps.

var userPromptChunks []string
var expressionMappings []*ExpressionMapping
Expand Down
201 changes: 174 additions & 27 deletions pkg/workflow/engine_agent_import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ func TestClaudeEngineWithoutAgentFile(t *testing.T) {
}
}

// TestCodexEngineWithAgentFromImports tests that codex engine prepends agent file content to prompt
// TestCodexEngineWithAgentFromImports tests that codex engine does NOT handle agent file natively
// and instead relies on the compiler to include the agent file content in prompt.txt
func TestCodexEngineWithAgentFromImports(t *testing.T) {
engine := NewCodexEngine()
workflowData := &WorkflowData{
Expand All @@ -205,23 +206,23 @@ func TestCodexEngineWithAgentFromImports(t *testing.T) {

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

// Check that agent content extraction is present
if !strings.Contains(stepContent, `AGENT_CONTENT="$(awk`) {
t.Errorf("Expected agent content extraction in codex command, got:\n%s", stepContent)
// Codex does not handle the agent file natively — no awk or AGENT_CONTENT variable
// juggling should appear in the step.
if strings.Contains(stepContent, "AGENT_CONTENT") {
t.Errorf("Codex must NOT handle agent file natively (AGENT_CONTENT found in step); the compiler handles it:\n%s", stepContent)
}

// Check that agent file path is referenced with quoted GITHUB_WORKSPACE prefix
if !strings.Contains(stepContent, `"${GITHUB_WORKSPACE}/.github/agents/test-agent.md"`) {
t.Errorf("Expected agent file path with quoted GITHUB_WORKSPACE prefix in codex command, got:\n%s", stepContent)
if strings.Contains(stepContent, "awk") {
t.Errorf("Codex must NOT invoke awk for agent file reading (found in step); the compiler handles it:\n%s", stepContent)
}

// Check that agent content is prepended to prompt using printf
if !strings.Contains(stepContent, `INSTRUCTION="$(printf`) {
t.Errorf("Expected printf with INSTRUCTION in codex command, got:\n%s", stepContent)
// The engine still reads the standard prompt.txt (which has agent content prepended by the compiler).
if !strings.Contains(stepContent, `INSTRUCTION="$(cat "$GH_AW_PROMPT")"`) {
t.Errorf("Expected standard prompt.txt reading in codex command, got:\n%s", stepContent)
}

if !strings.Contains(stepContent, "$AGENT_CONTENT") {
t.Errorf("Expected $AGENT_CONTENT variable in codex command, got:\n%s", stepContent)
// The engine reports that it does not support native agent file handling.
if engine.SupportsNativeAgentFile() {
t.Errorf("Codex engine should return false for SupportsNativeAgentFile()")
}
}

Expand Down Expand Up @@ -254,6 +255,154 @@ func TestCodexEngineWithoutAgentFile(t *testing.T) {
}
}

// TestCodexEngineDoesNotSupportNativeAgentFile verifies that the Codex engine declares
// it does not handle agent files natively, so the compiler knows to prepend the agent file
// content to prompt.txt during the activation job instead.
func TestCodexEngineDoesNotSupportNativeAgentFile(t *testing.T) {
engine := NewCodexEngine()
if engine.SupportsNativeAgentFile() {
t.Errorf("Codex engine should return false for SupportsNativeAgentFile(); the compiler handles agent file injection")
}
}

// TestCodexEngineAWFWithAgentFileReadsPromptTxt verifies that when an agent file is used
// with the firewall (AWF) enabled, the codex command reads from prompt.txt (not from a
// AGENT_CONTENT shell variable). The compiler prepends the agent file content to prompt.txt
// in the activation job.
func TestCodexEngineAWFWithAgentFileReadsPromptTxt(t *testing.T) {
engine := NewCodexEngine()

agentSandbox := &AgentSandboxConfig{Type: SandboxTypeAWF}
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "codex",
},
AgentFile: ".github/agents/test-agent.md",
SandboxConfig: &SandboxConfig{
Agent: agentSandbox,
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/agent-stdio.log")
if len(steps) == 0 {
t.Fatal("Expected at least one step")
}

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

// No AGENT_CONTENT shell variable anywhere in the step.
if strings.Contains(stepContent, "AGENT_CONTENT") {
t.Errorf("AGENT_CONTENT must not appear in the Codex AWF step; compiler handles agent file injection:\n%s", stepContent)
}
if strings.Contains(stepContent, "awk") {
t.Errorf("awk must not appear in the Codex AWF step; compiler handles agent file injection:\n%s", stepContent)
}

// The container command must still read from prompt.txt.
if !strings.Contains(stepContent, `INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) {
t.Errorf("Expected codex to read from prompt.txt in AWF mode, got:\n%s", stepContent)
}
}

// TestGeminiEngineDoesNotSupportNativeAgentFile verifies that the Gemini engine declares
// it does not handle agent files natively, so the compiler knows to prepend the agent file
// content to prompt.txt during the activation job instead.
func TestGeminiEngineDoesNotSupportNativeAgentFile(t *testing.T) {
engine := NewGeminiEngine()
if engine.SupportsNativeAgentFile() {
t.Errorf("Gemini engine should return false for SupportsNativeAgentFile(); the compiler handles agent file injection")
}
}

// TestGeminiEngineWithAgentFromImports tests that Gemini engine does NOT handle agent file natively
// and instead relies on the compiler to include the agent file content in prompt.txt
func TestGeminiEngineWithAgentFromImports(t *testing.T) {
engine := NewGeminiEngine()
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "gemini",
},
AgentFile: ".github/agents/test-agent.md",
Tools: map[string]any{},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")

// GetExecutionSteps returns a settings step + execution step for Gemini
if len(steps) == 0 {
t.Fatal("Expected at least one execution step")
}

// Combine all step content for inspection
var allContent strings.Builder
for _, step := range steps {
allContent.WriteString(strings.Join([]string(step), "\n"))
allContent.WriteString("\n")
}
combined := allContent.String()

// Gemini does not handle the agent file natively — no awk or AGENT_CONTENT
if strings.Contains(combined, "AGENT_CONTENT") {
t.Errorf("Gemini must NOT handle agent file natively (AGENT_CONTENT found in steps); the compiler handles it:\n%s", combined)
}
if strings.Contains(combined, "awk") {
t.Errorf("Gemini must NOT invoke awk for agent file reading (found in steps); the compiler handles it:\n%s", combined)
}

// The execution step must read the prompt from prompt.txt
if !strings.Contains(combined, `"$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) {
t.Errorf("Expected Gemini to read from prompt.txt, got:\n%s", combined)
}
}

// TestGeminiEngineAWFWithAgentFileReadsPromptTxt verifies that when an agent file is used
// with the firewall (AWF) enabled, the gemini command reads from prompt.txt (not from a
// AGENT_CONTENT shell variable). The compiler prepends the agent file content to prompt.txt
// in the activation job.
func TestGeminiEngineAWFWithAgentFileReadsPromptTxt(t *testing.T) {
engine := NewGeminiEngine()

agentSandbox := &AgentSandboxConfig{Type: SandboxTypeAWF}
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
ID: "gemini",
},
AgentFile: ".github/agents/test-agent.md",
SandboxConfig: &SandboxConfig{
Agent: agentSandbox,
},
Tools: map[string]any{},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/agent-stdio.log")
if len(steps) == 0 {
t.Fatal("Expected at least one step")
}

var allContent strings.Builder
for _, step := range steps {
allContent.WriteString(strings.Join([]string(step), "\n"))
allContent.WriteString("\n")
}
combined := allContent.String()

// No AGENT_CONTENT shell variable anywhere in the steps.
if strings.Contains(combined, "AGENT_CONTENT") {
t.Errorf("AGENT_CONTENT must not appear in the Gemini AWF steps; compiler handles agent file injection:\n%s", combined)
}
if strings.Contains(combined, "awk") {
t.Errorf("awk must not appear in the Gemini AWF steps; compiler handles agent file injection:\n%s", combined)
}

// The command must still read from prompt.txt.
if !strings.Contains(combined, `"$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`) {
t.Errorf("Expected gemini to read from prompt.txt in AWF mode, got:\n%s", combined)
}
}

// TestAgentFileValidation tests compile-time validation of agent file existence
func TestAgentFileValidation(t *testing.T) {
// Create a temporary directory structure that mimics a repository
Expand Down Expand Up @@ -353,15 +502,16 @@ This is a test agent file.
})
}

// TestInvalidAgentFilePathGeneratesFailingStep tests that engines that handle agent files
// natively emit a clearly-failing step (rather than silently skipping execution) when the
// agent file path contains shell metacharacters.
// Engines that do NOT support native agent files (e.g. Claude) rely on the compiler's
// validateAgentFile to reject malicious paths at compile time instead.
// TestInvalidAgentFilePathGeneratesFailingStep tests that engines that do NOT handle agent files
// natively (Claude, Codex, Gemini) rely on the compiler's validateAgentFile to reject malicious
// paths at compile time. Engine steps should proceed normally and never reference agent file paths.
func TestInvalidAgentFilePathGeneratesFailingStep(t *testing.T) {
maliciousPath := `.github/agents/a";id;"b.md`

t.Run("codex_emits_failing_step_for_invalid_path", func(t *testing.T) {
// Codex does not handle agent files natively; path validation is done by the compiler
// at compile time (validateAgentFile). The engine step should proceed normally and never
// reference the agent file path directly.
t.Run("codex_ignores_agent_path_in_step_for_invalid_path", func(t *testing.T) {
Comment on lines +505 to +514
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

TestInvalidAgentFilePathGeneratesFailingStep no longer asserts a failing step (it now asserts the engine ignores the agent path), but the name/comments still imply the compiler rejects malicious paths at compile time. Consider (1) renaming this test to reflect the new behavior, and (2) adding an explicit subtest that calls compiler.validateAgentFile with maliciousPath and asserts it returns the expected "invalid characters" error, so the compile-time rejection described here is actually covered.

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

Copilot uses AI. Check for mistakes.
engine := NewCodexEngine()
workflowData := &WorkflowData{
Name: "test-workflow",
Expand All @@ -370,18 +520,15 @@ func TestInvalidAgentFilePathGeneratesFailingStep(t *testing.T) {
steps := engine.GetExecutionSteps(workflowData, "/tmp/test.log")

if len(steps) != 1 {
t.Fatalf("Expected exactly 1 failing step, got %d", len(steps))
t.Fatalf("Expected exactly 1 step, got %d", len(steps))
}
content := strings.Join([]string(steps[0]), "\n")
if !strings.Contains(content, "exit 1") {
t.Errorf("Expected failing step with 'exit 1', got:\n%s", content)
}
if !strings.Contains(content, "Error") {
t.Errorf("Expected error message in failing step, got:\n%s", content)
// Must NOT reference the malicious path at all in the generated step
if strings.Contains(content, maliciousPath) {
t.Errorf("Codex step must not reference the agent file path directly, got:\n%s", content)
}
// Must NOT invoke awk (that would mean the path was used for real execution)
if strings.Contains(content, "awk") {
t.Errorf("Failing step must not invoke awk with the invalid path, got:\n%s", content)
t.Errorf("Codex step must not invoke awk for agent file reading, got:\n%s", content)
}
})

Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/gemini_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func NewGeminiEngine() *GeminiEngine {
supportsMaxTurns: false,
supportsMaxContinuations: false, // Gemini CLI does not support --max-autopilot-continues-style continuation mode
supportsWebSearch: false,
supportsNativeAgentFile: false, // Gemini does not support agent file natively; the compiler prepends the agent file content to prompt.txt
llmGatewayPort: constants.GeminiLLMGatewayPort,
},
}
Expand Down
Loading