Skip to content

Commit 15d7e27

Browse files
authored
Fix invalid JSON escapes in GitHub remote MCP gateway config generation (#41864)
1 parent c8b18f9 commit 15d7e27

6 files changed

Lines changed: 81 additions & 15 deletions

File tree

.github/workflows/github-remote-mcp-auth-test.lock.yml

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

actions/setup/js/start_mcp_gateway.cjs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,38 @@ function sleep(ms) {
6565
return new Promise(resolve => setTimeout(resolve, ms));
6666
}
6767

68+
/**
69+
* Builds targeted context for a JSON.parse error so logs can point at the likely key.
70+
*
71+
* @param {string} jsonText
72+
* @param {string} parseErrorMessage
73+
* @returns {{line: number, column: number, lineText: string, key: string | null} | null}
74+
*/
75+
function getJSONParseErrorContext(jsonText, parseErrorMessage) {
76+
const posMatch = parseErrorMessage.match(/position\s+(\d+)/i);
77+
if (!posMatch) {
78+
return null;
79+
}
80+
81+
const pos = Number(posMatch[1]);
82+
if (!Number.isFinite(pos) || pos < 0) {
83+
return null;
84+
}
85+
86+
const safePos = Math.min(pos, Math.max(0, jsonText.length - 1));
87+
const before = jsonText.slice(0, safePos);
88+
const line = before.split("\n").length;
89+
const lineStart = before.lastIndexOf("\n") + 1;
90+
const lineEnd = jsonText.indexOf("\n", safePos);
91+
const resolvedLineEnd = lineEnd === -1 ? jsonText.length : lineEnd;
92+
const lineText = jsonText.slice(lineStart, resolvedLineEnd);
93+
const column = safePos - lineStart + 1;
94+
const keyMatch = lineText.match(/"([^"]+)"\s*:/);
95+
const key = keyMatch ? keyMatch[1] : null;
96+
97+
return { line, column, lineText, key };
98+
}
99+
68100
/**
69101
* Normalizes GH_AW_OTLP_IF_MISSING to a supported mode.
70102
* @param {string | undefined} value
@@ -367,7 +399,16 @@ async function main() {
367399
core.error("ERROR: Configuration is not valid JSON");
368400
core.error("");
369401
core.error("JSON validation error:");
370-
core.error(/** @type {Error} */ err.message);
402+
const parseMessage = /** @type {Error} */ err.message;
403+
core.error(parseMessage);
404+
const parseContext = getJSONParseErrorContext(mcpConfig, parseMessage);
405+
if (parseContext) {
406+
core.error(`Likely offending location: line ${parseContext.line}, column ${parseContext.column}`);
407+
if (parseContext.key) {
408+
core.error(`Likely offending key: ${parseContext.key}`);
409+
}
410+
core.error(`Context line: ${parseContext.lineText}`);
411+
}
371412
core.error("");
372413
core.error("Configuration content:");
373414
const lines = mcpConfig.split("\n");
@@ -909,5 +950,6 @@ module.exports = {
909950
getOTLPIfMissingMode,
910951
hasNonEmptyOTLPHeaders,
911952
isOTLPIfMissingIgnore,
953+
getJSONParseErrorContext,
912954
resolveCopilotConfigPaths,
913955
};

actions/setup/js/start_mcp_gateway.test.cjs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2-
import { applyOTLPIgnoreIfMissing, detectEngineType, getOTLPIfMissingMode, hasNonEmptyOTLPHeaders, resolveCopilotConfigPaths } from "./start_mcp_gateway.cjs";
2+
import { applyOTLPIgnoreIfMissing, detectEngineType, getJSONParseErrorContext, getOTLPIfMissingMode, hasNonEmptyOTLPHeaders, resolveCopilotConfigPaths } from "./start_mcp_gateway.cjs";
33

44
describe("start_mcp_gateway OTLP if-missing helpers", () => {
55
let originalWarning;
@@ -197,3 +197,27 @@ describe("start_mcp_gateway detectEngineType", () => {
197197
expect(detectEngineType(configDir, env, existsSync)).toBe("copilot");
198198
});
199199
});
200+
201+
describe("start_mcp_gateway getJSONParseErrorContext", () => {
202+
it("extracts line/column and key for invalid escape values", () => {
203+
const invalidConfig = `{
204+
"mcpServers": {
205+
"github": {
206+
"env": {
207+
"GITHUB_HOST": "\\https://github.com"
208+
}
209+
}
210+
}
211+
}`;
212+
let parseErrorMessage = "";
213+
try {
214+
JSON.parse(invalidConfig);
215+
} catch (err) {
216+
parseErrorMessage = /** @type {Error} */ err.message;
217+
}
218+
const context = getJSONParseErrorContext(invalidConfig, parseErrorMessage);
219+
expect(context).toBeTruthy();
220+
expect(context?.key).toBe("GITHUB_HOST");
221+
expect(context?.lineText).toContain(`"GITHUB_HOST"`);
222+
});
223+
});

pkg/workflow/github_remote_config_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ func TestRenderGitHubMCPRemoteConfig(t *testing.T) {
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"`,
@@ -104,8 +104,8 @@ func TestRenderGitHubMCPRemoteConfig(t *testing.T) {
104104
`"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"`,
@@ -132,8 +132,8 @@ func TestRenderGitHubMCPRemoteConfig(t *testing.T) {
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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ This is a test workflow for GitHub remote mode configuration.
258258
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 {
@@ -460,7 +460,7 @@ This tests that GITHUB_PERSONAL_ACCESS_TOKEN is exported and passed to Docker.
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_renderer_github.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ func RenderGitHubMCPRemoteConfig(yaml *strings.Builder, options GitHubMCPRemoteO
305305
yaml,
306306
" ",
307307
"env",
308-
buildGitHubMCPEnvVars("\\${GITHUB_MCP_SERVER_TOKEN}", "\\${GITHUB_SERVER_URL}", false, false, ""),
308+
buildGitHubMCPEnvVars("${GITHUB_MCP_SERVER_TOKEN}", "${GITHUB_SERVER_URL}", false, false, ""),
309309
hasGuardPolicies,
310310
)
311311
}

0 commit comments

Comments
 (0)