diff --git a/.github/workflows/smoke-gemini.lock.yml b/.github/workflows/smoke-gemini.lock.yml index 2f17a3cd946..7db68a38c97 100644 --- a/.github/workflows/smoke-gemini.lock.yml +++ b/.github/workflows/smoke-gemini.lock.yml @@ -835,7 +835,7 @@ jobs: echo "$BASE_CONFIG" > "$SETTINGS" fi env: - GH_AW_GEMINI_BASE_CONFIG: {"context":{"includeDirectories":["/tmp/"]},"tools":{"core":["glob","grep_search","list_directory","read_file","read_many_files","replace","run_shell_command","write_file"]}} + GH_AW_GEMINI_BASE_CONFIG: '{"context":{"includeDirectories":["/tmp/"]},"tools":{"core":["glob","grep_search","list_directory","read_file","read_many_files","replace","run_shell_command","write_file"]}}' - name: Execute Gemini CLI id: agentic_execution run: | @@ -1070,7 +1070,7 @@ jobs: echo "$BASE_CONFIG" > "$SETTINGS" fi env: - GH_AW_GEMINI_BASE_CONFIG: {"context":{"includeDirectories":["/tmp/"]},"tools":{"core":["glob","grep_search","list_directory","read_file","read_many_files","run_shell_command(cat)","run_shell_command(grep)","run_shell_command(head)","run_shell_command(jq)","run_shell_command(ls)","run_shell_command(tail)","run_shell_command(wc)"]}} + GH_AW_GEMINI_BASE_CONFIG: '{"context":{"includeDirectories":["/tmp/"]},"tools":{"core":["glob","grep_search","list_directory","read_file","read_many_files","run_shell_command(cat)","run_shell_command(grep)","run_shell_command(head)","run_shell_command(jq)","run_shell_command(ls)","run_shell_command(tail)","run_shell_command(wc)"]}}' - name: Execute Gemini CLI if: always() && steps.detection_guard.outputs.run_detection == 'true' id: detection_agentic_execution diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 33e3adb4d4d..5561b85a715 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -210,13 +210,31 @@ func FormatStepWithCommandAndEnv(stepLines []string, command string, env map[str for _, key := range envKeys { value := env[key] - stepLines = append(stepLines, fmt.Sprintf(" %s: %s", key, value)) + stepLines = append(stepLines, fmt.Sprintf(" %s: %s", key, yamlStringValue(value))) } } return stepLines } +// yamlStringValue returns a YAML-safe representation of a string value. +// If the value starts with a YAML flow indicator ('{' or '[') or other characters +// that would cause it to be misinterpreted by YAML parsers, it wraps the value +// in single quotes. Any embedded single quotes are escaped by doubling them (' becomes ”). +func yamlStringValue(value string) string { + if len(value) == 0 { + return value + } + // Values starting with YAML flow indicators need quoting to be treated as strings. + // '{' would be parsed as a YAML flow mapping, '[' as a YAML flow sequence. + first := value[0] + if first != '{' && first != '[' { + return value + } + // Single-quote the value, escaping any embedded single quotes by doubling them. + return "'" + strings.ReplaceAll(value, "'", "''") + "'" +} + // FilterEnvForSecrets filters environment variables to only include allowed secrets. // This is a security measure to ensure that only necessary secrets are passed to the execution step. // diff --git a/pkg/workflow/engine_helpers_test.go b/pkg/workflow/engine_helpers_test.go index 0f9a4c0a728..b33475755fb 100644 --- a/pkg/workflow/engine_helpers_test.go +++ b/pkg/workflow/engine_helpers_test.go @@ -363,3 +363,81 @@ func TestGetNpmBinPathSetup_NoGorootDoesNotBreakChain(t *testing.T) { t.Errorf("Expected command chain to continue when GOROOT is empty, got: %q", result) } } + +func TestYamlStringValue(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "plain string unchanged", + input: "hello world", + expected: "hello world", + }, + { + name: "empty string unchanged", + input: "", + expected: "", + }, + { + name: "github actions expression unchanged", + input: "${{ secrets.TOKEN }}", + expected: "${{ secrets.TOKEN }}", + }, + { + name: "json object gets single-quoted", + input: `{"key":"value"}`, + expected: `'{"key":"value"}'`, + }, + { + name: "json array gets single-quoted", + input: `["a","b"]`, + expected: `'["a","b"]'`, + }, + { + name: "json object with embedded single quote gets escaped", + input: `{"key":"it's"}`, + expected: `'{"key":"it''s"}'`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := yamlStringValue(tt.input) + if result != tt.expected { + t.Errorf("yamlStringValue(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestFormatStepWithCommandAndEnvYAMLSafe(t *testing.T) { + t.Run("json env var is single-quoted for valid YAML", func(t *testing.T) { + stepLines := []string{" - name: Test step"} + env := map[string]string{ + "MY_JSON": `{"key":"value","nested":{"a":1}}`, + } + result := FormatStepWithCommandAndEnv(stepLines, "echo test", env) + output := strings.Join(result, "\n") + + // The JSON value must be single-quoted so YAML treats it as a string + if !strings.Contains(output, `MY_JSON: '{"key":"value","nested":{"a":1}}'`) { + t.Errorf("Expected single-quoted JSON in env, got:\n%s", output) + } + }) + + t.Run("github expression env var is not quoted", func(t *testing.T) { + stepLines := []string{" - name: Test step"} + env := map[string]string{ + "MY_TOKEN": "${{ secrets.TOKEN }}", + } + result := FormatStepWithCommandAndEnv(stepLines, "echo test", env) + output := strings.Join(result, "\n") + + // GitHub Actions expressions should not be wrapped in extra quotes + if !strings.Contains(output, "MY_TOKEN: ${{ secrets.TOKEN }}") { + t.Errorf("Expected unquoted github expression in env, got:\n%s", output) + } + }) +} diff --git a/pkg/workflow/gemini_engine_test.go b/pkg/workflow/gemini_engine_test.go index b6ce867c88b..16c64ce3a99 100644 --- a/pkg/workflow/gemini_engine_test.go +++ b/pkg/workflow/gemini_engine_test.go @@ -513,4 +513,16 @@ func TestGenerateGeminiSettingsStep(t *testing.T) { assert.Contains(t, content, "write_file", "Should include write_file for edit tool") assert.Contains(t, content, "replace", "Should include replace for edit tool") }) + + t.Run("GH_AW_GEMINI_BASE_CONFIG env var is single-quoted for valid YAML", func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "test-workflow", + Tools: map[string]any{}, + } + step := engine.generateGeminiSettingsStep(workflowData) + content := strings.Join(step, "\n") + + // The JSON value must be single-quoted so YAML doesn't treat it as an object + assert.Contains(t, content, "GH_AW_GEMINI_BASE_CONFIG: '", "JSON env var value must be single-quoted for valid YAML") + }) }