Skip to content

Commit f210dc2

Browse files
Copilotpelikhan
Copilot
and
pelikhan
authored
Fix JSON escaping in MCP string-map section rendering
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 4c211d1 commit f210dc2

13 files changed

Lines changed: 55 additions & 35 deletions

‎pkg/workflow/copilot_engine_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,7 +1457,7 @@ func TestCopilotEngineRenderGitHubMCPConfig(t *testing.T) {
14571457
`"type": "stdio",`,
14581458
`"container": "ghcr.io/github/github-mcp-server:` + string(constants.DefaultGitHubMCPServerVersion) + `"`,
14591459
`"env": {`,
1460-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
1460+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
14611461
`},`,
14621462
},
14631463
},
@@ -1472,7 +1472,7 @@ func TestCopilotEngineRenderGitHubMCPConfig(t *testing.T) {
14721472
`"type": "stdio",`,
14731473
`"container": "ghcr.io/github/github-mcp-server:v1.2.3"`,
14741474
`"env": {`,
1475-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
1475+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
14761476
`}`,
14771477
},
14781478
},

‎pkg/workflow/copilot_github_mcp_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestRenderGitHubCopilotMCPConfig_AllowedTools(t *testing.T) {
2929
`"type": "stdio"`,
3030
`"container": "ghcr.io/github/github-mcp-server:` + string(constants.DefaultGitHubMCPServerVersion) + `"`,
3131
`"env": {`,
32-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
32+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
3333
},
3434
unexpectedContent: []string{},
3535
},

‎pkg/workflow/engine_helpers_github_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ func TestRenderGitHubMCPDockerConfig(t *testing.T) {
5454
`"type": "stdio"`,
5555
`"container": "ghcr.io/github/github-mcp-server:latest"`,
5656
`"env": {`,
57-
`"GITHUB_HOST": "\${GITHUB_SERVER_URL}"`,
58-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
57+
`"GITHUB_HOST": "\\${GITHUB_SERVER_URL}"`,
58+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
5959
`"GITHUB_TOOLSETS": "default"`,
6060
},
6161
notFound: []string{

‎pkg/workflow/github_remote_config_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,14 @@ func TestRenderGitHubMCPRemoteConfig(t *testing.T) {
7474
`"type": "http"`,
7575
`"url": "https://api.githubcopilot.com/mcp/"`,
7676
`"headers": {`,
77-
`"Authorization": "Bearer \${GITHUB_PERSONAL_ACCESS_TOKEN}"`,
77+
`"Authorization": "Bearer \\${GITHUB_PERSONAL_ACCESS_TOKEN}"`,
7878
`"X-MCP-Toolsets": "default"`,
7979
`"tools": [`,
8080
`"list_issues"`,
8181
`"create_issue"`,
8282
`"env": {`,
83-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
84-
`"GITHUB_HOST": "\${GITHUB_SERVER_URL}"`,
83+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
84+
`"GITHUB_HOST": "\\${GITHUB_SERVER_URL}"`,
8585
},
8686
notExpected: []string{
8787
`"X-MCP-Readonly"`,
@@ -101,11 +101,11 @@ func TestRenderGitHubMCPRemoteConfig(t *testing.T) {
101101
`"type": "http"`,
102102
`"url": "https://api.githubcopilot.com/mcp/"`,
103103
`"headers": {`,
104-
`"Authorization": "Bearer \${GITHUB_PERSONAL_ACCESS_TOKEN}"`,
104+
`"Authorization": "Bearer \\${GITHUB_PERSONAL_ACCESS_TOKEN}"`,
105105
`"X-MCP-Toolsets": "all"`,
106106
`"env": {`,
107-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
108-
`"GITHUB_HOST": "\${GITHUB_SERVER_URL}"`,
107+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
108+
`"GITHUB_HOST": "\\${GITHUB_SERVER_URL}"`,
109109
},
110110
notExpected: []string{
111111
`"X-MCP-Readonly"`,
@@ -125,15 +125,15 @@ func TestRenderGitHubMCPRemoteConfig(t *testing.T) {
125125
`"type": "http"`,
126126
`"url": "https://api.githubcopilot.com/mcp/"`,
127127
`"headers": {`,
128-
`"Authorization": "Bearer \${GITHUB_PERSONAL_ACCESS_TOKEN}"`,
128+
`"Authorization": "Bearer \\${GITHUB_PERSONAL_ACCESS_TOKEN}"`,
129129
`"X-MCP-Readonly": "true"`,
130130
`"X-MCP-Toolsets": "repos"`,
131131
`"tools": [`,
132132
`"list_repositories"`,
133133
`"get_repository"`,
134134
`"env": {`,
135-
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`,
136-
`"GITHUB_HOST": "\${GITHUB_SERVER_URL}"`,
135+
`"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`,
136+
`"GITHUB_HOST": "\\${GITHUB_SERVER_URL}"`,
137137
},
138138
notExpected: []string{},
139139
},

‎pkg/workflow/github_remote_mode_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,10 @@ This is a test workflow for GitHub remote mode configuration.
255255
}
256256
// For Copilot engine, check for new ${} syntax
257257
if tt.engineType == "copilot" {
258-
if !strings.Contains(lockContent, `"Authorization": "Bearer \${GITHUB_PERSONAL_ACCESS_TOKEN}"`) {
258+
if !strings.Contains(lockContent, `"Authorization": "Bearer \\${GITHUB_PERSONAL_ACCESS_TOKEN}"`) {
259259
t.Errorf("Expected Authorization header with ${GITHUB_PERSONAL_ACCESS_TOKEN} syntax but didn't find it in:\n%s", lockContent)
260260
}
261-
if !strings.Contains(lockContent, `"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`) {
261+
if !strings.Contains(lockContent, `"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`) {
262262
t.Errorf("Expected env section with GITHUB_PERSONAL_ACCESS_TOKEN passthrough but didn't find it in:\n%s", lockContent)
263263
}
264264
} else {
@@ -455,12 +455,12 @@ This tests that GITHUB_PERSONAL_ACCESS_TOKEN is exported and passed to Docker.
455455
}
456456

457457
// Check that the MCP config still uses the ${} syntax
458-
if !strings.Contains(lockContent, `"Authorization": "Bearer \${GITHUB_PERSONAL_ACCESS_TOKEN}"`) {
458+
if !strings.Contains(lockContent, `"Authorization": "Bearer \\${GITHUB_PERSONAL_ACCESS_TOKEN}"`) {
459459
t.Errorf("Expected Authorization header with ${GITHUB_PERSONAL_ACCESS_TOKEN} syntax but didn't find it in lock file")
460460
}
461461

462462
// Check that the env section still defines the variable
463-
if !strings.Contains(lockContent, `"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}"`) {
463+
if !strings.Contains(lockContent, `"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}"`) {
464464
t.Errorf("Expected env section with GITHUB_PERSONAL_ACCESS_TOKEN but didn't find it in lock file")
465465
}
466466
}

‎pkg/workflow/mcp_http_headers_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,9 @@ func TestRenderSharedMCPConfig_HTTPWithHeaderSecrets(t *testing.T) {
211211
// Check that headers use env var references instead of secret expressions
212212
expectedHeaderChecks := []string{
213213
`"headers": {`,
214-
`"DD_API_KEY": "\${DD_API_KEY}"`,
215-
`"DD_APPLICATION_KEY": "\${DD_APPLICATION_KEY}"`,
216-
`"DD_SITE": "\${DD_SITE}"`,
214+
`"DD_API_KEY": "\\${DD_API_KEY}"`,
215+
`"DD_APPLICATION_KEY": "\\${DD_APPLICATION_KEY}"`,
216+
`"DD_SITE": "\\${DD_SITE}"`,
217217
}
218218

219219
for _, expected := range expectedHeaderChecks {
@@ -225,9 +225,9 @@ func TestRenderSharedMCPConfig_HTTPWithHeaderSecrets(t *testing.T) {
225225
// Check that env passthrough section is present
226226
expectedEnvChecks := []string{
227227
`"env": {`,
228-
`"DD_API_KEY": "\${DD_API_KEY}"`,
229-
`"DD_APPLICATION_KEY": "\${DD_APPLICATION_KEY}"`,
230-
`"DD_SITE": "\${DD_SITE}"`,
228+
`"DD_API_KEY": "\\${DD_API_KEY}"`,
229+
`"DD_APPLICATION_KEY": "\\${DD_APPLICATION_KEY}"`,
230+
`"DD_SITE": "\\${DD_SITE}"`,
231231
}
232232

233233
for _, expected := range expectedEnvChecks {

‎pkg/workflow/mcp_renderer_section_helpers.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package workflow
22

33
import (
4+
"encoding/json"
45
"fmt"
56
"strings"
67
)
@@ -11,7 +12,9 @@ func writeJSONStringMapEntries(yaml *strings.Builder, values map[string]string,
1112
if i == len(values)-1 {
1213
comma = ""
1314
}
14-
fmt.Fprintf(yaml, "%s\"%s\": \"%s\"%s\n", indent, key, values[key], comma)
15+
encodedKey, _ := json.Marshal(key) //nolint:jsonmarshalignorederror // marshaling a string cannot fail
16+
encodedValue, _ := json.Marshal(values[key]) //nolint:jsonmarshalignorederror // marshaling a string cannot fail
17+
fmt.Fprintf(yaml, "%s%s: %s%s\n", indent, encodedKey, encodedValue, comma)
1518
}
1619
}
1720

‎pkg/workflow/mcp_renderer_section_helpers_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package workflow
44

55
import (
6+
"encoding/json"
67
"strings"
78
"testing"
89
)
@@ -25,6 +26,22 @@ func TestWriteJSONStringMapSection(t *testing.T) {
2526
}
2627
}
2728

29+
func TestWriteJSONStringMapSectionEscapesKeysAndValues(t *testing.T) {
30+
var output strings.Builder
31+
32+
writeJSONStringMapSection(&output, " ", "env", map[string]string{
33+
`A"key`: "line1\nline2\\end",
34+
}, false)
35+
36+
var parsed map[string]map[string]string
37+
if err := json.Unmarshal([]byte("{\n"+output.String()+"}"), &parsed); err != nil {
38+
t.Fatalf("expected valid JSON section, got error: %v\noutput:\n%s", err, output.String())
39+
}
40+
if parsed["env"][`A"key`] != "line1\nline2\\end" {
41+
t.Fatalf("unexpected parsed value: %#v", parsed["env"])
42+
}
43+
}
44+
2845
func TestWriteTOMLInlineStringMapSection(t *testing.T) {
2946
var output strings.Builder
3047

‎pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,8 @@ jobs:
480480
"type": "stdio",
481481
"container": "ghcr.io/github/github-mcp-server:v1.4.0",
482482
"env": {
483-
"GITHUB_HOST": "\${GITHUB_SERVER_URL}",
484-
"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}",
483+
"GITHUB_HOST": "\\${GITHUB_SERVER_URL}",
484+
"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}",
485485
"GITHUB_READ_ONLY": "1",
486486
"GITHUB_TOOLSETS": "context,repos,issues,pull_requests"
487487
},

‎pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,8 @@ jobs:
480480
"type": "stdio",
481481
"container": "ghcr.io/github/github-mcp-server:v1.4.0",
482482
"env": {
483-
"GITHUB_HOST": "\${GITHUB_SERVER_URL}",
484-
"GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}",
483+
"GITHUB_HOST": "\\${GITHUB_SERVER_URL}",
484+
"GITHUB_PERSONAL_ACCESS_TOKEN": "\\${GITHUB_MCP_SERVER_TOKEN}",
485485
"GITHUB_READ_ONLY": "1",
486486
"GITHUB_TOOLSETS": "context,repos,issues,pull_requests"
487487
},

0 commit comments

Comments
 (0)