Skip to content
Closed
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
24 changes: 24 additions & 0 deletions .github/workflows/smoke-codex.lock.yml

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

4 changes: 3 additions & 1 deletion pkg/workflow/add_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ func (c *Compiler) buildCreateOutputAddCommentJob(data *WorkflowData, mainJobNam
if len(data.SafeOutputs.AddComments.AllowedReasons) > 0 {
reasonsJSON, err := json.Marshal(data.SafeOutputs.AddComments.AllowedReasons)
if err == nil {
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_ALLOWED_REASONS: %q\n", string(reasonsJSON)))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(string(reasonsJSON))
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_ALLOWED_REASONS: '%s'\n", escapedJSON))
}
}
// Add environment variables for the URLs from other safe output jobs if they exist
Expand Down
8 changes: 6 additions & 2 deletions pkg/workflow/compiler_activation_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec
steps = append(steps, " env:\n")
// Pass commands as JSON array
commandsJSON, _ := json.Marshal(data.Command)
steps = append(steps, fmt.Sprintf(" GH_AW_COMMANDS: %q\n", string(commandsJSON)))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(string(commandsJSON))
steps = append(steps, fmt.Sprintf(" GH_AW_COMMANDS: '%s'\n", escapedJSON))
steps = append(steps, " with:\n")
steps = append(steps, " script: |\n")
steps = append(steps, generateGitHubScriptWithRequire("check_command_position.cjs"))
Expand Down Expand Up @@ -434,7 +436,9 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate
if err != nil {
compilerActivationJobsLog.Printf("Warning: failed to serialize messages config for activation job: %v", err)
} else if messagesJSON != "" {
steps = append(steps, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: %q\n", messagesJSON))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(messagesJSON)
steps = append(steps, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: '%s'\n", escapedJSON))
}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/workflow/compiler_safe_outputs_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,9 +635,10 @@ func (c *Compiler) addProjectHandlerManagerConfigEnvVar(steps *[]string, data *W
consolidatedSafeOutputsLog.Printf("Failed to marshal project handler config: %v", err)
return
}
// Escape the JSON for YAML (handle quotes and special chars)
configStr := string(configJSON)
*steps = append(*steps, fmt.Sprintf(" GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG: %q\n", configStr))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
// This prevents injection when JSON values contain special characters
escapedJSON := escapeSingleQuote(string(configJSON))
*steps = append(*steps, fmt.Sprintf(" GH_AW_SAFE_OUTPUTS_PROJECT_HANDLER_CONFIG: '%s'\n", escapedJSON))
}
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/workflow/compiler_safe_outputs_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,9 @@ func (c *Compiler) buildJobLevelSafeOutputEnvVars(data *WorkflowData, workflowID
if err != nil {
consolidatedSafeOutputsJobLog.Printf("Warning: failed to serialize messages config: %v", err)
} else if messagesJSON != "" {
envVars["GH_AW_SAFE_OUTPUT_MESSAGES"] = fmt.Sprintf("%q", messagesJSON)
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(messagesJSON)
envVars["GH_AW_SAFE_OUTPUT_MESSAGES"] = fmt.Sprintf("'%s'", escapedJSON)
}
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/workflow/missing_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ func (c *Compiler) buildCreateOutputMissingDataJob(data *WorkflowData, mainJobNa
if len(data.SafeOutputs.MissingData.Labels) > 0 {
labelsJSON, err := json.Marshal(data.SafeOutputs.MissingData.Labels)
if err == nil {
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_MISSING_DATA_LABELS: %q\n", string(labelsJSON)))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(string(labelsJSON))
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_MISSING_DATA_LABELS: '%s'\n", escapedJSON))
missingDataLog.Printf("labels: %v", data.SafeOutputs.MissingData.Labels)
}
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/workflow/missing_tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ func (c *Compiler) buildCreateOutputMissingToolJob(data *WorkflowData, mainJobNa
if len(data.SafeOutputs.MissingTool.Labels) > 0 {
labelsJSON, err := json.Marshal(data.SafeOutputs.MissingTool.Labels)
if err == nil {
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_MISSING_TOOL_LABELS: %q\n", string(labelsJSON)))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(string(labelsJSON))
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_MISSING_TOOL_LABELS: '%s'\n", escapedJSON))
missingToolLog.Printf("labels: %v", data.SafeOutputs.MissingTool.Labels)
}
}
Expand Down
16 changes: 12 additions & 4 deletions pkg/workflow/notify_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa
if len(data.SafeOutputs.MissingTool.Labels) > 0 {
labelsJSON, err := json.Marshal(data.SafeOutputs.MissingTool.Labels)
if err == nil {
missingToolEnvVars = append(missingToolEnvVars, fmt.Sprintf(" GH_AW_MISSING_TOOL_LABELS: %q\n", string(labelsJSON)))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(string(labelsJSON))
missingToolEnvVars = append(missingToolEnvVars, fmt.Sprintf(" GH_AW_MISSING_TOOL_LABELS: '%s'\n", escapedJSON))
}
}

Expand Down Expand Up @@ -164,7 +166,9 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa
if err != nil {
notifyCommentLog.Printf("Warning: failed to serialize messages config for agent failure handler: %v", err)
} else if messagesJSON != "" {
agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: %q\n", messagesJSON))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(messagesJSON)
agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: '%s'\n", escapedJSON))
}
}

Expand Down Expand Up @@ -225,15 +229,19 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa
if err != nil {
notifyCommentLog.Printf("Warning: failed to serialize messages config: %v", err)
} else if messagesJSON != "" {
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: %q\n", messagesJSON))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(messagesJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: '%s'\n", escapedJSON))
}
}

// Pass safe output job information for link generation
if len(safeOutputJobNames) > 0 {
safeOutputJobsJSON, jobURLEnvVars := buildSafeOutputJobsEnvVars(safeOutputJobNames)
if safeOutputJobsJSON != "" {
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_JOBS: %q\n", safeOutputJobsJSON))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(safeOutputJobsJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_JOBS: '%s'\n", escapedJSON))
customEnvVars = append(customEnvVars, jobURLEnvVars...)
notifyCommentLog.Printf("Added safe output jobs info for %d job(s)", len(safeOutputJobNames))
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/workflow/safe_outputs_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ func (c *Compiler) buildStandardSafeOutputEnvVars(data *WorkflowData, targetRepo
if err != nil {
safeOutputsEnvLog.Printf("Warning: failed to serialize messages config: %v", err)
} else if messagesJSON != "" {
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: %q\n", messagesJSON))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
escapedJSON := escapeSingleQuote(messagesJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_MESSAGES: '%s'\n", escapedJSON))
}
}

Expand Down
9 changes: 4 additions & 5 deletions pkg/workflow/update_project_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ func (c *Compiler) buildUpdateProjectJob(data *WorkflowData, mainJobName string)
if err != nil {
return nil, fmt.Errorf("failed to marshal views configuration: %w", err)
}
// lgtm[go/unsafe-quoting] - This generates YAML environment variable declarations, not shell commands.
// The %q format specifier properly escapes the JSON string for YAML syntax. There is no shell injection
// risk because this value is set as an environment variable in the GitHub Actions YAML configuration,
// not executed as shell code.
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_VIEWS: %q\n", string(viewsJSON)))
// Escape single quotes and backslashes for safe embedding in YAML single-quoted strings
// This prevents injection when JSON values contain special characters
escapedJSON := escapeSingleQuote(string(viewsJSON))
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_VIEWS: '%s'\n", escapedJSON))
}

jobCondition := BuildSafeOutputType("update_project")
Expand Down
161 changes: 161 additions & 0 deletions pkg/workflow/update_project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,164 @@ func TestUpdateProjectJob_Permissions(t *testing.T) {
require.NotEmpty(t, job.Permissions, "Job should have permissions")
assert.Contains(t, job.Permissions, "contents: read", "Should have contents: read permission")
}

func TestUpdateProjectJob_ViewsEscaping(t *testing.T) {
tests := []struct {
name string
views []ProjectView
shouldEscape bool
checkContent string
}{
{
name: "simple views without special characters",
views: []ProjectView{
{Name: "status", Layout: "board", Filter: "status:Todo"},
{Name: "priority", Layout: "table", Filter: "priority:High"},
},
shouldEscape: false,
checkContent: "status",
},
{
name: "views with single quotes in filter",
views: []ProjectView{
{Name: "broken", Layout: "board", Filter: "label:\"can't fix\""},
{Name: "issues", Layout: "table", Description: "It's broken"},
},
shouldEscape: true,
checkContent: `\'`,
},
{
name: "views with backslashes in description",
views: []ProjectView{
{Name: "regex", Layout: "board", Description: `Pattern: \d+\.\d+`},
{Name: "path", Layout: "table", Filter: `path:C:\\Windows`},
},
shouldEscape: true,
checkContent: `\\`,
},
{
name: "views with mixed special characters",
views: []ProjectView{
{Name: "complex", Layout: "board", Description: `It's a "test" with \backslash`},
},
shouldEscape: true,
checkContent: `\\`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := NewCompiler(false, "", "test")

workflowData := &WorkflowData{
Name: "test-workflow",
SafeOutputs: &SafeOutputsConfig{
UpdateProjects: &UpdateProjectConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{
Max: 10,
},
Views: tt.views,
},
},
}

job, err := compiler.buildUpdateProjectJob(workflowData, "main")
require.NoError(t, err)
require.NotNil(t, job)

// Find the step containing GH_AW_PROJECT_VIEWS
var viewsEnvVar string
for _, step := range job.Steps {
if strings.Contains(step, "GH_AW_PROJECT_VIEWS:") {
viewsEnvVar = step
break
}
}

require.NotEmpty(t, viewsEnvVar, "Should contain GH_AW_PROJECT_VIEWS environment variable")

// Verify that the JSON is properly escaped
if tt.shouldEscape {
// Should use single-quoted YAML string
assert.Contains(t, viewsEnvVar, "GH_AW_PROJECT_VIEWS: '", "Should use single-quoted YAML string")

// Verify the expected escape sequences are present
assert.Contains(t, viewsEnvVar, tt.checkContent, "Should contain escaped characters")
}

// Verify the environment variable is properly formatted as YAML
assert.Contains(t, viewsEnvVar, "GH_AW_PROJECT_VIEWS:", "Should contain environment variable key")
})
}
}

func TestUpdateProjectJob_ViewsNoInjection(t *testing.T) {
// Test that malicious input cannot break out of the YAML string
compiler := NewCompiler(false, "", "test")

maliciousViews := []ProjectView{
{Name: "injection", Layout: "board", Filter: "'; echo 'injected'; echo '"},
{Name: "path", Layout: "table", Description: `\'; rm -rf /; echo '`},
}

workflowData := &WorkflowData{
Name: "test-workflow",
SafeOutputs: &SafeOutputsConfig{
UpdateProjects: &UpdateProjectConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{
Max: 10,
},
Views: maliciousViews,
},
},
}

job, err := compiler.buildUpdateProjectJob(workflowData, "main")
require.NoError(t, err)
require.NotNil(t, job)

// Find the step containing GH_AW_PROJECT_VIEWS
var viewsEnvVar string
for _, step := range job.Steps {
if strings.Contains(step, "GH_AW_PROJECT_VIEWS:") {
viewsEnvVar = step
break
}
}

require.NotEmpty(t, viewsEnvVar, "Should contain GH_AW_PROJECT_VIEWS environment variable")

// Verify that all single quotes and backslashes are properly escaped
// The environment variable should be wrapped in single quotes
assert.Contains(t, viewsEnvVar, "GH_AW_PROJECT_VIEWS: '", "Should use single-quoted YAML string")

// Count opening quotes
openQuotes := strings.Count(viewsEnvVar, "GH_AW_PROJECT_VIEWS: '")
assert.Equal(t, 1, openQuotes, "Should have exactly one opening quote for the environment variable")

// Verify all single quotes in the JSON are escaped
lines := strings.Split(viewsEnvVar, "\n")
for _, line := range lines {
if strings.Contains(line, "GH_AW_PROJECT_VIEWS:") {
// Extract the value part after the colon
parts := strings.SplitN(line, "GH_AW_PROJECT_VIEWS: '", 2)
if len(parts) == 2 {
value := parts[1]
// Remove the trailing quote
value = strings.TrimSuffix(value, "'")

// Verify backslashes are escaped (doubled)
if strings.Contains(value, `\`) {
// Should contain escaped backslashes
assert.Contains(t, value, `\\`, "Backslashes should be escaped")
}

// Verify single quotes are escaped
if strings.Contains(value, "'") {
// Should contain escaped quotes
assert.Contains(t, value, `\'`, "Single quotes should be escaped")
}
}
}
}
}