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
10 changes: 5 additions & 5 deletions pkg/workflow/update_project_job.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package workflow

import (
"encoding/base64"
"encoding/json"
"fmt"
)
Expand Down Expand Up @@ -44,11 +45,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)))
// Base64 encode the JSON to eliminate any quoting concerns
// This prevents potential injection even if the value is misused in shell contexts
viewsBase64 := base64.StdEncoding.EncodeToString(viewsJSON)
customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PROJECT_VIEWS: %s\n", viewsBase64))
}

jobCondition := BuildSafeOutputType("update_project")
Expand Down
188 changes: 188 additions & 0 deletions pkg/workflow/update_project_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package workflow

import (
"encoding/base64"
"encoding/json"
"strings"
"testing"

Expand Down Expand Up @@ -324,3 +326,189 @@ 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_ViewsConfigurationBase64(t *testing.T) {
tests := []struct {
name string
views []map[string]any
expectViews bool
description string
}{
{
name: "no views configured",
views: nil,
expectViews: false,
description: "Should not set GH_AW_PROJECT_VIEWS when no views are configured",
},
{
name: "empty views array",
views: []map[string]any{},
expectViews: false,
description: "Should not set GH_AW_PROJECT_VIEWS when views array is empty",
},
{
name: "single view with simple name",
views: []map[string]any{
{"name": "My View", "id": "123"},
},
expectViews: true,
description: "Should base64 encode views configuration with simple names",
},
{
name: "view with special characters including single quotes",
views: []map[string]any{
{"name": "User's View", "id": "456", "description": "Contains 'quotes'"},
},
expectViews: true,
description: "Should safely handle views with single quotes via base64 encoding",
},
{
name: "view with double quotes and backslashes",
views: []map[string]any{
{"name": "Test\"View", "path": "\\path\\to\\view"},
},
expectViews: true,
description: "Should safely handle views with double quotes and backslashes via base64",
},
{
name: "multiple views with mixed special characters",
views: []map[string]any{
{"name": "View 1", "id": "1"},
{"name": "User's \"View\"", "id": "2"},
{"name": "Complex\\Path'View", "id": "3"},
},
expectViews: true,
description: "Should safely handle multiple views with various special characters",
},
}

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, tt.description)
require.NotNil(t, job, tt.description)

// Join all steps to search for environment variable
allSteps := strings.Join(job.Steps, "\n")

if tt.expectViews {
// Should contain the GH_AW_PROJECT_VIEWS environment variable
assert.Contains(t, allSteps, "GH_AW_PROJECT_VIEWS:", "Should set GH_AW_PROJECT_VIEWS when views are configured")

// Extract the base64 value from the steps
lines := strings.Split(allSteps, "\n")
var viewsBase64 string
for _, line := range lines {
if strings.Contains(line, "GH_AW_PROJECT_VIEWS:") {
parts := strings.SplitN(line, ":", 2)
if len(parts) == 2 {
viewsBase64 = strings.TrimSpace(parts[1])
break
}
}
}

require.NotEmpty(t, viewsBase64, "Should have base64-encoded views value")

// Verify the base64 value can be decoded back to the original JSON
decodedBytes, err := base64.StdEncoding.DecodeString(viewsBase64)
require.NoError(t, err, "Base64 value should be valid")

var decodedViews []map[string]any
err = json.Unmarshal(decodedBytes, &decodedViews)
require.NoError(t, err, "Decoded value should be valid JSON")

// Verify the decoded views match the original
assert.Equal(t, tt.views, decodedViews, "Decoded views should match original configuration")

// Security check: Verify no raw single quotes in the environment variable value
// This ensures the base64 encoding prevents any quoting issues
assert.NotContains(t, viewsBase64, "'", "Base64 value should not contain single quotes")
assert.NotContains(t, viewsBase64, "\"", "Base64 value should not contain double quotes")
} else {
// Should not contain GH_AW_PROJECT_VIEWS when no views configured
assert.NotContains(t, allSteps, "GH_AW_PROJECT_VIEWS:", "Should not set GH_AW_PROJECT_VIEWS when no views are configured")
}
})
}
}

func TestUpdateProjectJob_Base64EncodingPreventsSQLInjection(t *testing.T) {
// Test that base64 encoding prevents potential injection attacks
compiler := NewCompiler(false, "", "test")

maliciousViews := []map[string]any{
{
"name": "'; DROP TABLE projects; --",
"id": "injection-test",
},
{
"name": "${INJECTION}",
"id": "var-expansion-test",
},
{
"name": "`rm -rf /`",
"id": "command-injection-test",
},
}

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, "Should handle malicious input without error")
require.NotNil(t, job)

allSteps := strings.Join(job.Steps, "\n")

// Verify malicious strings are not present in raw form
assert.NotContains(t, allSteps, "DROP TABLE", "SQL injection attempt should be encoded")
assert.NotContains(t, allSteps, "rm -rf", "Command injection attempt should be encoded")

// Verify the base64 value is present
assert.Contains(t, allSteps, "GH_AW_PROJECT_VIEWS:", "Environment variable should be set")

// Extract and verify the base64 value contains only safe characters
lines := strings.Split(allSteps, "\n")
for _, line := range lines {
if strings.Contains(line, "GH_AW_PROJECT_VIEWS:") {
parts := strings.SplitN(line, ":", 2)
if len(parts) == 2 {
viewsBase64 := strings.TrimSpace(parts[1])
// Base64 should only contain alphanumeric, +, /, and = characters
for _, char := range viewsBase64 {
assert.True(t,
(char >= 'A' && char <= 'Z') ||
(char >= 'a' && char <= 'z') ||
(char >= '0' && char <= '9') ||
char == '+' || char == '/' || char == '=',
"Base64 value should only contain safe characters")
}
break
}
}
}
}