diff --git a/actions/setup/js/mount_mcp_as_cli.cjs b/actions/setup/js/mount_mcp_as_cli.cjs index 32a1c764d05..3acaf78eaf5 100644 --- a/actions/setup/js/mount_mcp_as_cli.cjs +++ b/actions/setup/js/mount_mcp_as_cli.cjs @@ -609,7 +609,9 @@ async function main() { } core.info(` Found ${tools.length} tool(s)`); - // Cache the tool list + // Cache the tool list. This file only contains tool name/description/schema + // metadata returned by the gateway; it does not contain the API key or any + // other secret, so world-readable permissions (0o644) are acceptable here. try { fs.writeFileSync(toolsFile, JSON.stringify(tools, null, 2), { mode: 0o644 }); } catch (err) { @@ -618,13 +620,31 @@ async function main() { // Write the CLI wrapper script using the container-accessible URL const scriptPath = path.join(CLI_BIN_DIR, name); + let scriptFd; try { - fs.writeFileSync(scriptPath, generateCLIWrapperScript(name, containerUrl, toolsFile, apiKey, bridgeScript), { mode: 0o755 }); + // Owner-only permissions: the wrapper script embeds the plaintext gateway API key, + // so it must not be world- or group-readable (matches chmod 600 used elsewhere for + // this same credential, e.g. convert_gateway_config_copilot.sh). + // Note: writeFileSync(mode) only applies when creating a new file; for existing files, + // force mode with fchmodSync so prior permissive modes (e.g., 0o755) are corrected. + scriptFd = fs.openSync(scriptPath, "w", 0o700); + fs.fchmodSync(scriptFd, 0o700); + fs.writeFileSync(scriptFd, generateCLIWrapperScript(name, containerUrl, toolsFile, apiKey, bridgeScript), "utf8"); + fs.closeSync(scriptFd); + scriptFd = undefined; mountedServers.push(name); mountedServerTools.push({ name, tools }); core.info(` ✓ Mounted as: ${scriptPath}`); } catch (err) { core.warning(` Failed to write CLI wrapper for ${name}: ${getErrorMessage(err)}`); + } finally { + if (scriptFd !== undefined) { + try { + fs.closeSync(scriptFd); + } catch { + // Ignore close errors in cleanup path; main error already reported above. + } + } } } diff --git a/actions/setup/js/mount_mcp_as_cli.test.cjs b/actions/setup/js/mount_mcp_as_cli.test.cjs index d8b60858c19..8bc37491be5 100644 --- a/actions/setup/js/mount_mcp_as_cli.test.cjs +++ b/actions/setup/js/mount_mcp_as_cli.test.cjs @@ -1,6 +1,7 @@ // @ts-check -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import fs from "fs"; +import http from "http"; import os from "os"; import path from "path"; @@ -328,3 +329,93 @@ describe("mount_mcp_as_cli.cjs", () => { expect(warnings[1]).toContain("stopping empty tools/list retries"); }); }); + +describe("mount_mcp_as_cli.cjs main() file permissions", () => { + /** @type {string | undefined} */ + let tempDir; + /** @type {http.Server | undefined} */ + let server; + + afterEach(async () => { + if (server) { + await new Promise(resolve => server.close(resolve)); + server = undefined; + } + if (tempDir) { + // The bin directory is locked to 0o555 by main(); restore write permissions + // so the temp directory can be removed during cleanup. + const binDir = path.join(tempDir, "gh-aw/mcp-cli/bin"); + if (fs.existsSync(binDir)) { + fs.chmodSync(binDir, 0o755); + } + fs.rmSync(tempDir, { recursive: true, force: true }); + tempDir = undefined; + } + delete process.env.RUNNER_TEMP; + delete process.env.MCP_GATEWAY_API_KEY; + delete process.env.MCP_GATEWAY_DOMAIN; + delete process.env.MCP_GATEWAY_PORT; + delete global.core; + vi.resetModules(); + }); + + it("rewrites an existing 0o755 CLI wrapper script to owner-only permissions (0o700)", async () => { + // Minimal fake MCP server that answers initialize / notifications/initialized / tools/list + server = http.createServer((req, res) => { + let data = ""; + req.on("data", chunk => (data += chunk)); + req.on("end", () => { + const parsed = JSON.parse(data); + if (parsed.method === "tools/list") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ jsonrpc: "2.0", id: parsed.id, result: { tools: [{ name: "echo" }] } })); + } else { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ jsonrpc: "2.0", id: parsed.id, result: {} })); + } + }); + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-cli-mount-test-")); + process.env.RUNNER_TEMP = tempDir; + process.env.MCP_GATEWAY_API_KEY = "super-secret-gateway-key"; + delete process.env.MCP_GATEWAY_DOMAIN; + delete process.env.MCP_GATEWAY_PORT; + + const manifestDir = path.join(tempDir, "gh-aw/mcp-cli"); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.writeFileSync(path.join(manifestDir, "manifest.json"), JSON.stringify({ servers: [{ name: "testserver", url: `http://127.0.0.1:${port}/mcp` }] }), "utf8"); + + const binDir = path.join(tempDir, "gh-aw/mcp-cli/bin"); + fs.mkdirSync(binDir, { recursive: true }); + const scriptPath = path.join(binDir, "testserver"); + fs.writeFileSync(scriptPath, "#!/usr/bin/env bash\necho preexisting\n", { mode: 0o755 }); + fs.chmodSync(scriptPath, 0o755); + + const infos = []; + const warnings = []; + global.core = { + info: msg => infos.push(msg), + warning: msg => warnings.push(msg), + addPath: () => {}, + setOutput: () => {}, + }; + + vi.resetModules(); + const mod = await import("./mount_mcp_as_cli.cjs?t=" + Date.now()); + + await mod.main(); + + expect(fs.existsSync(scriptPath)).toBe(true); + + const mode = fs.statSync(scriptPath).mode & 0o777; + expect(mode).toBe(0o700); + expect(mode).not.toBe(0o755); + + const scriptContent = fs.readFileSync(scriptPath, "utf8"); + expect(scriptContent).toContain("super-secret-gateway-key"); + }); +});