From cd3fe4940aae0c2e1b34115baebbc36689fd8b14 Mon Sep 17 00:00:00 2001 From: Menghua1 Date: Thu, 7 Aug 2025 11:11:13 +0800 Subject: [PATCH 1/2] Add JSON escape to variables before syncing to remote Add JSON escape to variables before syncing to remote Add JSON escape to variables before syncing to remote Add JSON escape to variables before syncing to remote Add JSON escape to variables before syncing to remote --- cli/azd/pkg/azdo/pipeline.go | 18 ++++++++++++++++++ cli/azd/pkg/pipeline/github_provider.go | 15 +++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/cli/azd/pkg/azdo/pipeline.go b/cli/azd/pkg/azdo/pipeline.go index 65ff6cff449..687fd487155 100644 --- a/cli/azd/pkg/azdo/pipeline.go +++ b/cli/azd/pkg/azdo/pipeline.go @@ -5,6 +5,7 @@ package azdo import ( "context" + "encoding/json" "fmt" "strings" @@ -116,6 +117,7 @@ func CreatePipeline( if err != nil { return nil, err } + EscapeBuildDefinitionVariables(buildDefinitionVariables) definition.Variables = buildDefinitionVariables definition, err := client.UpdateDefinition(ctx, build.UpdateDefinitionArgs{ Definition: definition, @@ -140,6 +142,7 @@ func CreatePipeline( return nil, err } + EscapeBuildDefinitionVariables(createDefinitionArgs.Definition.Variables) newBuildDefinition, err := client.CreateDefinition(ctx, *createDefinitionArgs) if err != nil { return nil, err @@ -148,6 +151,21 @@ func CreatePipeline( return newBuildDefinition, nil } +func EscapeBuildDefinitionVariables(vars *map[string]build.BuildDefinitionVariable) { + for key, variable := range *vars { + if variable.Value != nil { + original := *variable.Value + b, _ := json.Marshal(original) + s := string(b) + if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { + s = s[1 : len(s)-1] + } + variable.Value = &s + (*vars)[key] = variable + } + } +} + func getDefinitionVariables( env *environment.Environment, credentials *entraid.AzureCredentials, diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index b441a39cee9..b02580620d3 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -979,6 +979,8 @@ func (p *GitHubCiProvider) configurePipeline( } } + JsonEscapeVariables(toBeSetSecrets, toBeSetVariables) + // set the new variables and secrets for key, value := range toBeSetSecrets { if err := p.ghCli.SetSecret(ctx, repoSlug, key, value); err != nil { @@ -999,6 +1001,19 @@ func (p *GitHubCiProvider) configurePipeline( }, nil } +func JsonEscapeVariables(vars ...map[string]string) { + for _, m := range vars { + for key, value := range m { + b, _ := json.Marshal(value) + s := string(b) + if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { + s = s[1 : len(s)-1] + } + m[key] = s + } + } +} + // workflow is the implementation for a CiPipeline for GitHub type workflow struct { repoDetails *gitRepositoryDetails From 13e8d57245c5b2bf4716cab005c544020308f0ae Mon Sep 17 00:00:00 2001 From: Menghua1 Date: Tue, 9 Dec 2025 17:39:35 +0800 Subject: [PATCH 2/2] Escape environment config values in pipeline manager before calling providers --- cli/azd/pkg/azdo/pipeline.go | 18 ---- cli/azd/pkg/pipeline/github_provider.go | 15 ---- cli/azd/pkg/pipeline/pipeline.go | 36 ++++++++ cli/azd/pkg/pipeline/pipeline_test.go | 106 ++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 33 deletions(-) diff --git a/cli/azd/pkg/azdo/pipeline.go b/cli/azd/pkg/azdo/pipeline.go index 687fd487155..65ff6cff449 100644 --- a/cli/azd/pkg/azdo/pipeline.go +++ b/cli/azd/pkg/azdo/pipeline.go @@ -5,7 +5,6 @@ package azdo import ( "context" - "encoding/json" "fmt" "strings" @@ -117,7 +116,6 @@ func CreatePipeline( if err != nil { return nil, err } - EscapeBuildDefinitionVariables(buildDefinitionVariables) definition.Variables = buildDefinitionVariables definition, err := client.UpdateDefinition(ctx, build.UpdateDefinitionArgs{ Definition: definition, @@ -142,7 +140,6 @@ func CreatePipeline( return nil, err } - EscapeBuildDefinitionVariables(createDefinitionArgs.Definition.Variables) newBuildDefinition, err := client.CreateDefinition(ctx, *createDefinitionArgs) if err != nil { return nil, err @@ -151,21 +148,6 @@ func CreatePipeline( return newBuildDefinition, nil } -func EscapeBuildDefinitionVariables(vars *map[string]build.BuildDefinitionVariable) { - for key, variable := range *vars { - if variable.Value != nil { - original := *variable.Value - b, _ := json.Marshal(original) - s := string(b) - if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { - s = s[1 : len(s)-1] - } - variable.Value = &s - (*vars)[key] = variable - } - } -} - func getDefinitionVariables( env *environment.Environment, credentials *entraid.AzureCredentials, diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go index 87fa1612fd0..ea86925ce77 100644 --- a/cli/azd/pkg/pipeline/github_provider.go +++ b/cli/azd/pkg/pipeline/github_provider.go @@ -979,8 +979,6 @@ func (p *GitHubCiProvider) configurePipeline( } } - JsonEscapeVariables(toBeSetSecrets, toBeSetVariables) - // set the new variables and secrets for key, value := range toBeSetSecrets { if err := p.ghCli.SetSecret(ctx, repoSlug, key, value); err != nil { @@ -1001,19 +999,6 @@ func (p *GitHubCiProvider) configurePipeline( }, nil } -func JsonEscapeVariables(vars ...map[string]string) { - for _, m := range vars { - for key, value := range m { - b, _ := json.Marshal(value) - s := string(b) - if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { - s = s[1 : len(s)-1] - } - m[key] = s - } - } -} - // workflow is the implementation for a CiPipeline for GitHub type workflow struct { repoDetails *gitRepositoryDetails diff --git a/cli/azd/pkg/pipeline/pipeline.go b/cli/azd/pkg/pipeline/pipeline.go index fd6cd8fb23b..624459a5982 100644 --- a/cli/azd/pkg/pipeline/pipeline.go +++ b/cli/azd/pkg/pipeline/pipeline.go @@ -5,6 +5,7 @@ package pipeline import ( "context" + "encoding/json" "fmt" "maps" "os" @@ -231,9 +232,44 @@ func mergeProjectVariablesAndSecrets( } } + // Escape values for safe transmission to pipeline providers. + // This ensures that values containing JSON-like content (e.g., `["api://..."]`) + // are properly escaped (e.g., `[\"api://...\"]`) before being sent to GitHub Actions or Azure DevOps. + // Without this, the remote pipeline may incorrectly parse the value as JSON instead of treating it as a string. + escapeValuesForPipeline(variables) + escapeValuesForPipeline(secrets) + return variables, secrets, nil } +// escapeValuesForPipeline applies JSON escaping to values to ensure they are correctly +// interpreted as strings by pipeline providers (GitHub Actions, Azure DevOps). +// +// When a value contains special characters (e.g., quotes, backslashes, brackets), it needs +// to be escaped before being sent to the remote pipeline. This function uses JSON marshaling +// to properly escape the value, then strips the outer quotes added by marshaling. +// +// Example: the value `["api://guid"]` becomes `[\"api://guid\"]` after escaping. +func escapeValuesForPipeline(values map[string]string) { + for key, value := range values { + // Use JSON marshaling to properly escape special characters + escapedBytes, err := json.Marshal(value) + if err != nil { + // If marshaling fails, keep the original value + continue + } + + escapedStr := string(escapedBytes) + // JSON marshaling wraps the string in quotes; remove them + // Example: json.Marshal("test") produces "\"test\"", we want just the inner content + if len(escapedStr) >= 2 && escapedStr[0] == '"' && escapedStr[len(escapedStr)-1] == '"' { + escapedStr = escapedStr[1 : len(escapedStr)-1] + } + + values[key] = escapedStr + } +} + const ( gitHubDisplayName string = "GitHub" gitHubCode = "github" diff --git a/cli/azd/pkg/pipeline/pipeline_test.go b/cli/azd/pkg/pipeline/pipeline_test.go index cab72f86112..6bd7e14e570 100644 --- a/cli/azd/pkg/pipeline/pipeline_test.go +++ b/cli/azd/pkg/pipeline/pipeline_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_ConfigOptions_SecretsAndVars(t *testing.T) { @@ -46,3 +47,108 @@ func Test_ConfigOptions_SecretsAndVars(t *testing.T) { assert.Equal(t, expectedVariables, variables) assert.Equal(t, expectedSecrets, secrets) } + +// Test_ConfigOptions_EscapedValues tests that JSON-escaped string values are preserved +// when merging project variables and secrets. +// This addresses the issue where values like `["api://..."]` need to be escaped +// to `[\"api://...\"]` when sent to remote pipelines to prevent them from being +// interpreted as JSON arrays instead of strings. +func Test_ConfigOptions_EscapedValues(t *testing.T) { + projectVariables := []string{"AzureAd_TokenValidationParameters_ValidAudiences"} + projectSecrets := []string{} + + initialVariables := map[string]string{} + initialSecrets := map[string]string{} + + // This simulates a value that is read from config.json. + // After JSON unmarshaling, the value `"[\"api://...\"]"` becomes `["api://..."]` (backslashes consumed) + // We need to re-escape it before sending to the pipeline so it's treated as a string, not an array + env := map[string]string{ + "AzureAd_TokenValidationParameters_ValidAudiences": "[\"api://e935a748-8b59-4c26-a59c-9bcc83f5ab57\"]", + } + + variables, secrets, err := mergeProjectVariablesAndSecrets( + projectVariables, projectSecrets, initialVariables, initialSecrets, nil, env) + require.NoError(t, err) + + // After escaping, the value should have backslashes to prevent JSON parsing in the pipeline + // The value becomes: [\"api://e935a748-8b59-4c26-a59c-9bcc83f5ab57\"] + expectedVariables := map[string]string{ + "AzureAd_TokenValidationParameters_ValidAudiences": "[\\\"api://e935a748-8b59-4c26-a59c-9bcc83f5ab57\\\"]", + } + expectedSecrets := map[string]string{} + + assert.Equal(t, expectedVariables, variables) + assert.Equal(t, expectedSecrets, secrets) +} + +// Test_ConfigOptions_SimpleValues tests that simple string values are properly escaped +func Test_ConfigOptions_SimpleValues(t *testing.T) { + projectVariables := []string{"SIMPLE_VAR", "VAR_WITH_QUOTES"} + projectSecrets := []string{} + + initialVariables := map[string]string{} + initialSecrets := map[string]string{} + + env := map[string]string{ + "SIMPLE_VAR": "simple-value", + "VAR_WITH_QUOTES": "value with \"quotes\"", + } + + variables, secrets, err := mergeProjectVariablesAndSecrets( + projectVariables, projectSecrets, initialVariables, initialSecrets, nil, env) + require.NoError(t, err) + + // Simple values remain mostly the same, quotes get escaped + expectedVariables := map[string]string{ + "SIMPLE_VAR": "simple-value", + "VAR_WITH_QUOTES": "value with \\\"quotes\\\"", + } + expectedSecrets := map[string]string{} + + assert.Equal(t, expectedVariables, variables) + assert.Equal(t, expectedSecrets, secrets) +} + +// Test_escapeValuesForPipeline tests the escape function directly +func Test_escapeValuesForPipeline(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "JSON array string", + input: "[\"api://guid\"]", + expected: "[\\\"api://guid\\\"]", + }, + { + name: "Simple string", + input: "simple", + expected: "simple", + }, + { + name: "String with quotes", + input: "value with \"quotes\"", + expected: "value with \\\"quotes\\\"", + }, + { + name: "String with backslashes", + input: "path\\to\\file", + expected: "path\\\\to\\\\file", + }, + { + name: "Empty string", + input: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + values := map[string]string{"test": tt.input} + escapeValuesForPipeline(values) + assert.Equal(t, tt.expected, values["test"]) + }) + } +}