From 22c9d2c0fc0859c0cf65b180b4678591e6612501 Mon Sep 17 00:00:00 2001 From: sirmacik <127441966+sirmacik@users.noreply.github.com> Date: Mon, 11 May 2026 22:17:02 +0200 Subject: [PATCH 1/2] feat(codexcli-mcp): support codex-specific env_vars passthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex CLI supports per-server `env_vars = [...]` arrays that name shell env vars to pass through to the MCP server process — distinct from `env` (a literal name→value map). Use case: secrets like API keys or auth tokens you don't want literal-encoded into a committed `mcp.json`. Before this PR, rulesync had no way to express `env_vars` in the source schema. Users either: - Lost their env_vars on every `rulesync generate` regen - Or added them to other tools' files where they don't belong Add `env_vars` as a recognised optional field on `McpServerSchema` (alongside the existing `kiroAutoApprove`, `kiroAutoBlock`, etc. tool-specific flat fields). Strip it in `RulesyncMcp.getMcpServers()` so it does not leak into Claude Code, Kilo, OpenCode, Gemini CLI, Copilot, etc. — all of which consume `getMcpServers()`. The codex generator reads it directly from `getJson()` instead. Source: { "mcpServers": { "pal": { "type": "stdio", "command": "uvx", "args": ["pal-mcp-server"], "env_vars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY"] } } } Codex output (`~/.codex/config.toml` or project `.codex/config.toml`): [mcp_servers.pal] type = "stdio" command = "uvx" args = ["pal-mcp-server"] env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY"] All other tools' outputs: env_vars stripped, never written. Tests: - codexcli: env_vars passes through to TOML output - rulesync-mcp: getMcpServers() strips env_vars - rulesync-mcp: getJson() still exposes env_vars for the codex generator Docs: added new section under `.rulesync/mcp.json` reference documenting the field and its codex-only emission. --- docs/reference/file-formats.md | 25 ++++++++++++++ skills/rulesync/file-formats.md | 25 ++++++++++++++ src/features/mcp/codexcli-mcp.test.ts | 35 +++++++++++++++++++ src/features/mcp/rulesync-mcp.test.ts | 48 +++++++++++++++++++++++++++ src/features/mcp/rulesync-mcp.ts | 5 ++- src/types/mcp.ts | 5 +++ 6 files changed, 142 insertions(+), 1 deletion(-) diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index 8a3869d4e..d9c57b587 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -361,6 +361,31 @@ You can control which individual tools from an MCP server are enabled or disable - `enabledTools`: An array of tool names that should be explicitly enabled for this server. - `disabledTools`: An array of tool names that should be explicitly disabled for this server. +### Codex-specific: pass shell env vars to MCP servers (`env_vars`) + +Codex CLI supports a per-server `env_vars` array that names shell environment variables to inherit when launching the MCP server process. This is distinct from `env` (which is a literal `{name: value}` map) — `env_vars` is a list of names whose values come from the user's environment. + +```json +{ + "mcpServers": { + "pal": { + "type": "stdio", + "command": "uvx", + "args": [ + "--from", + "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", + "pal-mcp-server" + ], + "env_vars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] + } + } +} +``` + +- Emitted only into the codex CLI output (`~/.codex/config.toml` or project `.codex/config.toml`). +- Stripped from `getMcpServers()` so it does not leak into other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, etc. each ignore it). +- Use this for secrets and API keys you do not want to literal-encode into a committed `mcp.json`. + ## `.rulesync/.aiignore` or `.rulesyncignore` Rulesync supports a single ignore list that can live in either location below: diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index 8a3869d4e..d9c57b587 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -361,6 +361,31 @@ You can control which individual tools from an MCP server are enabled or disable - `enabledTools`: An array of tool names that should be explicitly enabled for this server. - `disabledTools`: An array of tool names that should be explicitly disabled for this server. +### Codex-specific: pass shell env vars to MCP servers (`env_vars`) + +Codex CLI supports a per-server `env_vars` array that names shell environment variables to inherit when launching the MCP server process. This is distinct from `env` (which is a literal `{name: value}` map) — `env_vars` is a list of names whose values come from the user's environment. + +```json +{ + "mcpServers": { + "pal": { + "type": "stdio", + "command": "uvx", + "args": [ + "--from", + "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", + "pal-mcp-server" + ], + "env_vars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] + } + } +} +``` + +- Emitted only into the codex CLI output (`~/.codex/config.toml` or project `.codex/config.toml`). +- Stripped from `getMcpServers()` so it does not leak into other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, etc. each ignore it). +- Use this for secrets and API keys you do not want to literal-encode into a committed `mcp.json`. + ## `.rulesync/.aiignore` or `.rulesyncignore` Rulesync supports a single ignore list that can live in either location below: diff --git a/src/features/mcp/codexcli-mcp.test.ts b/src/features/mcp/codexcli-mcp.test.ts index d4c8dabcd..9f87dd67b 100644 --- a/src/features/mcp/codexcli-mcp.test.ts +++ b/src/features/mcp/codexcli-mcp.test.ts @@ -565,6 +565,41 @@ fontSize = 14 expect(mcpServers["my-server"].disabled_tools).toEqual(["delete"]); }); + it("should emit codex-specific env_vars array from source", async () => { + // env_vars is a codex CLI-specific field that tells codex to pass + // through named shell env vars to the MCP server process. Source: + // "pal": { ..., "env_vars": ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"] } + // Output: + // [mcp_servers.pal] + // env_vars = ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"] + const jsonData = { + mcpServers: { + pal: { + type: "stdio", + command: "uvx", + args: ["pal-mcp-server"], + env_vars: ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"], + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const mcpServers = codexcliMcp.getToml().mcp_servers as any; + expect(mcpServers.pal.env_vars).toEqual(["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"]); + // smoke check that the serialized TOML contains the array + expect(codexcliMcp.getFileContent()).toContain("env_vars"); + }); + it("should convert enabledTools/disabledTools for multiple servers", async () => { const jsonData = { mcpServers: { diff --git a/src/features/mcp/rulesync-mcp.test.ts b/src/features/mcp/rulesync-mcp.test.ts index f787eebc2..1c8b012de 100644 --- a/src/features/mcp/rulesync-mcp.test.ts +++ b/src/features/mcp/rulesync-mcp.test.ts @@ -986,6 +986,54 @@ describe("RulesyncMcp", () => { }); }); + describe("getMcpServers field stripping", () => { + it("should strip codex-specific env_vars from getMcpServers output", () => { + // env_vars is codex-only; it must NOT leak into other tools' generated + // configs (claudecode, opencode, kilo, geminicli, etc.) which all + // consume getMcpServers(). The codex generator reads env_vars directly + // from getJson() instead. + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ + mcpServers: { + pal: { + type: "stdio", + command: "uvx", + args: ["pal-mcp-server"], + env_vars: ["OPENAI_API_KEY", "OPENROUTER_API_KEY"], + }, + }, + }), + }); + + const servers = rulesyncMcp.getMcpServers(); + + expect(servers.pal).toBeDefined(); + expect((servers.pal as any).command).toBe("uvx"); + expect((servers.pal as any).env_vars).toBeUndefined(); + }); + + it("should still expose env_vars via getJson() for the codex generator", () => { + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ + mcpServers: { + pal: { + type: "stdio", + command: "uvx", + env_vars: ["OPENAI_API_KEY"], + }, + }, + }), + }); + + const fromJson = rulesyncMcp.getJson().mcpServers.pal; + expect((fromJson as any).env_vars).toEqual(["OPENAI_API_KEY"]); + }); + }); + describe("integration and edge cases", () => { it("should handle large JSON structures", () => { const largeJsonData = { diff --git a/src/features/mcp/rulesync-mcp.ts b/src/features/mcp/rulesync-mcp.ts index b819d75b0..8ea1bad6f 100644 --- a/src/features/mcp/rulesync-mcp.ts +++ b/src/features/mcp/rulesync-mcp.ts @@ -145,7 +145,10 @@ export class RulesyncMcp extends RulesyncFile { return Object.fromEntries( entries.map(([serverName, serverConfig]) => { - return [serverName, omit(serverConfig, ["targets", "description", "exposed"])]; + // `env_vars` is codex-specific: the codex generator reads it directly + // from the unfiltered source JSON. Strip here so it does not leak + // into other tools' outputs. + return [serverName, omit(serverConfig, ["targets", "description", "exposed", "env_vars"])]; }), ); } diff --git a/src/types/mcp.ts b/src/types/mcp.ts index 9d091ec4a..0fb9732fa 100644 --- a/src/types/mcp.ts +++ b/src/types/mcp.ts @@ -17,6 +17,11 @@ export const McpServerSchema = z.looseObject({ tools: z.optional(z.array(z.string())), kiroAutoApprove: z.optional(z.array(z.string())), kiroAutoBlock: z.optional(z.array(z.string())), + // Codex CLI-specific: list of shell env var names that codex should pass + // through from the user's environment to the MCP server process. Only + // honoured by the codex generator; stripped by `RulesyncMcp.getMcpServers()` + // so it does not leak into other tools' configs. + env_vars: z.optional(z.array(z.string())), headers: z.optional(z.record(z.string(), z.string())), enabledTools: z.optional(z.array(z.string())), disabledTools: z.optional(z.array(z.string())), From ae7adb334cdb059d89fc94c5dc8661c9669e2fc1 Mon Sep 17 00:00:00 2001 From: sirmacik <127441966+sirmacik@users.noreply.github.com> Date: Mon, 11 May 2026 22:47:45 +0200 Subject: [PATCH 2/2] fix(mcp): make envVars truly codex-only; rename + close leak paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of #1623 surfaced two compounding issues: 1. **Leak (high)**: 6 generators bypass `RulesyncMcp.getMcpServers()` and pull mcpServers from the unfiltered source via `getJson()` or `getFileContent()`. The PR's stated guarantee that env_vars is codex-only was therefore false: - cline-mcp.ts (getFileContent) - cursor-mcp.ts (getJson) - factorydroid-mcp.ts (getJson) - geminicli-mcp.ts (getJson) - junie-mcp.ts (getFileContent) - rovodev-mcp.ts (getJson) Migrate each to `getMcpServers()` (cline/junie additionally need to JSON-serialize since they previously copied the raw file). The migration also closes a *separate* pre-existing leak of the rulesync-only fields `targets`/`description`/`exposed` through the same six generators (the cursor complex-config test was actually asserting this leaked behaviour — updated to reflect the correct stripped output). 2. **Naming (medium)**: the source field was `env_vars` (snake_case), breaking the MCP schema's camelCase convention used by `enabledTools`, `disabledTools`, `kiroAutoApprove`, `httpUrl`, etc. Rename to `envVars` and use the same emit-time rename pattern as `enabledTools` → `enabled_tools` in `convertToCodexFormat` / reverse in `convertFromCodexFormat`. Also harden `getMcpServers()` to tolerate missing/empty `mcpServers` in source — previously it threw on `Object.entries(undefined)`. Callers that read `getJson().mcpServers` via `isMcpServers()` relied on this resilience; centralising it removes the need for the guard at each call site. Tests: - new test in codex test file: source camelCase `envVars` → output snake_case `env_vars`; source key absent; defensive assertions on other surviving fields - new test: coexistence of `envVars` + `env` on the same server - new test: round-trip codex `env_vars` → toRulesyncMcp → `envVars` - new test in gemini test file: `envVars` is stripped from gemini output (representative leak-prevention test for a non-codex generator) - updated rulesync-mcp tests for the rename - updated cursor complex-config test (now asserts `targets` is correctly stripped instead of leaked) Docs: updated `docs/reference/file-formats.md` section to use `envVars` (camelCase) in the source example, show the snake_case codex output, and explicitly enumerate the non-codex tools the strip covers. --- docs/reference/file-formats.md | 24 ++++++--- skills/rulesync/file-formats.md | 24 ++++++--- src/features/mcp/cline-mcp.ts | 13 ++++- src/features/mcp/codexcli-mcp.test.ts | 75 +++++++++++++++++++++++--- src/features/mcp/codexcli-mcp.ts | 11 ++++ src/features/mcp/cursor-mcp.test.ts | 23 +++++++- src/features/mcp/cursor-mcp.ts | 6 ++- src/features/mcp/factorydroid-mcp.ts | 7 ++- src/features/mcp/geminicli-mcp.test.ts | 35 ++++++++++++ src/features/mcp/geminicli-mcp.ts | 5 +- src/features/mcp/junie-mcp.ts | 13 ++++- src/features/mcp/rovodev-mcp.ts | 6 ++- src/features/mcp/rulesync-mcp.test.ts | 16 +++--- src/features/mcp/rulesync-mcp.ts | 11 ++-- src/types/mcp.ts | 13 +++-- 15 files changed, 237 insertions(+), 45 deletions(-) diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index d9c57b587..b7a0d2705 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -361,9 +361,11 @@ You can control which individual tools from an MCP server are enabled or disable - `enabledTools`: An array of tool names that should be explicitly enabled for this server. - `disabledTools`: An array of tool names that should be explicitly disabled for this server. -### Codex-specific: pass shell env vars to MCP servers (`env_vars`) +### Codex-specific: pass shell env vars to MCP servers (`envVars`) -Codex CLI supports a per-server `env_vars` array that names shell environment variables to inherit when launching the MCP server process. This is distinct from `env` (which is a literal `{name: value}` map) — `env_vars` is a list of names whose values come from the user's environment. +Codex CLI supports a per-server array of shell env var names to inherit when launching the MCP server process. The source schema uses `envVars` (camelCase, matching the project convention used by sibling fields like `enabledTools`/`disabledTools`); the codex generator renames it to `env_vars` (snake_case) for codex's native `config.toml` format. + +This is distinct from `env` (which is a literal `{name: value}` map) — `envVars` is a list of names whose **values come from the user's environment at runtime**. Both fields may coexist on the same server. ```json { @@ -376,15 +378,25 @@ Codex CLI supports a per-server `env_vars` array that names shell environment va "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", "pal-mcp-server" ], - "env_vars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] + "envVars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] } } } ``` -- Emitted only into the codex CLI output (`~/.codex/config.toml` or project `.codex/config.toml`). -- Stripped from `getMcpServers()` so it does not leak into other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, etc. each ignore it). -- Use this for secrets and API keys you do not want to literal-encode into a committed `mcp.json`. +Generated `~/.codex/config.toml`: + +```toml +[mcp_servers.pal] +type = "stdio" +command = "uvx" +args = ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", "pal-mcp-server"] +env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] +``` + +- Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). +- Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. +- Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined — see codex documentation for the exact resolution rule. ## `.rulesync/.aiignore` or `.rulesyncignore` diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index d9c57b587..b7a0d2705 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -361,9 +361,11 @@ You can control which individual tools from an MCP server are enabled or disable - `enabledTools`: An array of tool names that should be explicitly enabled for this server. - `disabledTools`: An array of tool names that should be explicitly disabled for this server. -### Codex-specific: pass shell env vars to MCP servers (`env_vars`) +### Codex-specific: pass shell env vars to MCP servers (`envVars`) -Codex CLI supports a per-server `env_vars` array that names shell environment variables to inherit when launching the MCP server process. This is distinct from `env` (which is a literal `{name: value}` map) — `env_vars` is a list of names whose values come from the user's environment. +Codex CLI supports a per-server array of shell env var names to inherit when launching the MCP server process. The source schema uses `envVars` (camelCase, matching the project convention used by sibling fields like `enabledTools`/`disabledTools`); the codex generator renames it to `env_vars` (snake_case) for codex's native `config.toml` format. + +This is distinct from `env` (which is a literal `{name: value}` map) — `envVars` is a list of names whose **values come from the user's environment at runtime**. Both fields may coexist on the same server. ```json { @@ -376,15 +378,25 @@ Codex CLI supports a per-server `env_vars` array that names shell environment va "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", "pal-mcp-server" ], - "env_vars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] + "envVars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] } } } ``` -- Emitted only into the codex CLI output (`~/.codex/config.toml` or project `.codex/config.toml`). -- Stripped from `getMcpServers()` so it does not leak into other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, etc. each ignore it). -- Use this for secrets and API keys you do not want to literal-encode into a committed `mcp.json`. +Generated `~/.codex/config.toml`: + +```toml +[mcp_servers.pal] +type = "stdio" +command = "uvx" +args = ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", "pal-mcp-server"] +env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] +``` + +- Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). +- Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. +- Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined — see codex documentation for the exact resolution rule. ## `.rulesync/.aiignore` or `.rulesyncignore` diff --git a/src/features/mcp/cline-mcp.ts b/src/features/mcp/cline-mcp.ts index 3b5b0807d..bf7dcbd6b 100644 --- a/src/features/mcp/cline-mcp.ts +++ b/src/features/mcp/cline-mcp.ts @@ -59,11 +59,22 @@ export class ClineMcp extends ToolMcp { rulesyncMcp, validate = true, }: ToolMcpFromRulesyncMcpParams): ClineMcp { + // Preserve top-level fields ($schema, config, etc.) from the source + // JSON, but use getMcpServers() (not getJson().mcpServers) so + // rulesync-only fields and codex-only fields (`envVars`) are stripped + // before writing the cline config. + const json = rulesyncMcp.getJson(); + const fileContent = JSON.stringify( + { ...json, mcpServers: rulesyncMcp.getMcpServers() }, + null, + 2, + ); + return new ClineMcp({ outputRoot, relativeDirPath: this.getSettablePaths().relativeDirPath, relativeFilePath: this.getSettablePaths().relativeFilePath, - fileContent: rulesyncMcp.getFileContent(), + fileContent, validate, }); } diff --git a/src/features/mcp/codexcli-mcp.test.ts b/src/features/mcp/codexcli-mcp.test.ts index 9f87dd67b..37b019189 100644 --- a/src/features/mcp/codexcli-mcp.test.ts +++ b/src/features/mcp/codexcli-mcp.test.ts @@ -565,10 +565,12 @@ fontSize = 14 expect(mcpServers["my-server"].disabled_tools).toEqual(["delete"]); }); - it("should emit codex-specific env_vars array from source", async () => { - // env_vars is a codex CLI-specific field that tells codex to pass - // through named shell env vars to the MCP server process. Source: - // "pal": { ..., "env_vars": ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"] } + it("should rename source envVars (camelCase) to codex env_vars (snake_case)", async () => { + // The source schema uses `envVars` (camelCase) for consistency with + // `enabledTools`/`disabledTools`. The codex generator renames it to + // `env_vars` (snake_case) to match codex's native config.toml format. + // Source: + // "pal": { ..., "envVars": ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"] } // Output: // [mcp_servers.pal] // env_vars = ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"] @@ -578,7 +580,7 @@ fontSize = 14 type: "stdio", command: "uvx", args: ["pal-mcp-server"], - env_vars: ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"], + envVars: ["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"], }, }, }; @@ -595,9 +597,68 @@ fontSize = 14 }); const mcpServers = codexcliMcp.getToml().mcp_servers as any; + // Output uses snake_case (codex native). expect(mcpServers.pal.env_vars).toEqual(["OPENAI_API_KEY", "JIRA_PERSONAL_TOKEN"]); - // smoke check that the serialized TOML contains the array - expect(codexcliMcp.getFileContent()).toContain("env_vars"); + // Source key (camelCase) must NOT appear in codex output. + expect(mcpServers.pal.envVars).toBeUndefined(); + // Defensive: other fields survive. + expect(mcpServers.pal.command).toBe("uvx"); + expect(mcpServers.pal.args).toEqual(["pal-mcp-server"]); + }); + + it("should coexist envVars and env on the same server", async () => { + // `envVars` (list of names inherited from shell) and `env` (literal + // name→value map) are distinct concepts. Both must serialize correctly + // on the same server. + const jsonData = { + mcpServers: { + pal: { + type: "stdio", + command: "uvx", + args: ["pal-mcp-server"], + envVars: ["OPENAI_API_KEY"], + env: { LOG_LEVEL: "debug" }, + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const server = (codexcliMcp.getToml().mcp_servers as any).pal; + expect(server.env_vars).toEqual(["OPENAI_API_KEY"]); + expect(server.env).toEqual({ LOG_LEVEL: "debug" }); + }); + + it("should round-trip envVars through codex import", async () => { + // codex config.toml → toRulesyncMcp() → rulesync representation must + // expose `envVars` in source schema form (camelCase). + const tomlContent = `[mcp_servers.pal] +type = "stdio" +command = "uvx" +args = ["pal-mcp-server"] +env_vars = ["OPENAI_API_KEY"] +`; + const codexcliMcp = new CodexcliMcp({ + outputRoot: testDir, + relativeDirPath: ".codex", + relativeFilePath: "config.toml", + fileContent: tomlContent, + }); + + const rulesyncMcp = codexcliMcp.toRulesyncMcp(); + const json = JSON.parse(rulesyncMcp.getFileContent()); + + expect(json.mcpServers.pal.envVars).toEqual(["OPENAI_API_KEY"]); + expect(json.mcpServers.pal.env_vars).toBeUndefined(); }); it("should convert enabledTools/disabledTools for multiple servers", async () => { diff --git a/src/features/mcp/codexcli-mcp.ts b/src/features/mcp/codexcli-mcp.ts index 5afffce63..267c280e4 100644 --- a/src/features/mcp/codexcli-mcp.ts +++ b/src/features/mcp/codexcli-mcp.ts @@ -33,6 +33,11 @@ function convertFromCodexFormat(codexMcp: Record): McpServers { converted["enabledTools"] = value; } else if (key === "disabled_tools") { converted["disabledTools"] = value; + } else if (key === "env_vars") { + // codex stores env-var passthrough names in snake_case (`env_vars`); + // the rulesync source schema uses camelCase (`envVars`) for + // consistency with `enabledTools`/`disabledTools`/etc. + converted["envVars"] = value; } else { converted[key] = value; } @@ -58,6 +63,12 @@ function convertToCodexFormat(mcpServers: McpServers): Record { converted["enabled_tools"] = value; } else if (key === "disabledTools") { converted["disabled_tools"] = value; + } else if (key === "envVars") { + // Rename camelCase source `envVars` → snake_case `env_vars` + // for codex's native config.toml format. See `enabledTools` + // precedent above. `envVars` itself is stripped from + // getMcpServers() so non-codex tools never receive it. + converted["env_vars"] = value; } else { converted[key] = value; } diff --git a/src/features/mcp/cursor-mcp.test.ts b/src/features/mcp/cursor-mcp.test.ts index cf288fa10..b5273dab9 100644 --- a/src/features/mcp/cursor-mcp.test.ts +++ b/src/features/mcp/cursor-mcp.test.ts @@ -954,9 +954,28 @@ describe("CursorMcp", () => { validate: true, }); - expect(cursorMcp.getJson()).toEqual({ - mcpServers: rulesyncMcpData.mcpServers, + // `targets` is a rulesync-only field and is stripped from non-codex + // tool outputs by `RulesyncMcp.getMcpServers()`. The cursor output + // must contain everything else but not `targets`. + const json = cursorMcp.getJson() as any; + expect(json.mcpServers["complex-server"]).toEqual({ + command: "node", + args: ["complex-server.js", "--port", "3000"], + env: { + NODE_ENV: "production", + DEBUG: "mcp:*", + }, + }); + expect(json.mcpServers["python-server"]).toEqual({ + command: "python", + args: ["python-server.py"], + env: { + PYTHONPATH: "/usr/local/lib/python3.9/site-packages", + }, }); + // `targets` should be stripped. + expect(json.mcpServers["complex-server"].targets).toBeUndefined(); + expect(json.mcpServers["python-server"].targets).toBeUndefined(); }); it("should use custom outputRoot when provided", async () => { diff --git a/src/features/mcp/cursor-mcp.ts b/src/features/mcp/cursor-mcp.ts index 393f5b362..ff5df30c9 100644 --- a/src/features/mcp/cursor-mcp.ts +++ b/src/features/mcp/cursor-mcp.ts @@ -149,8 +149,10 @@ export class CursorMcp extends ToolMcp { ); } - const rulesyncJson = rulesyncMcp.getJson(); - const mcpServers = isMcpServers(rulesyncJson.mcpServers) ? rulesyncJson.mcpServers : {}; + // Use getMcpServers() (not getJson()) so rulesync-only fields and + // codex-only fields (`envVars`) are stripped before writing the + // cursor config. + const mcpServers = rulesyncMcp.getMcpServers(); const transformedServers = convertEnvToCursorFormat(mcpServers); const cursorConfig = { ...json, mcpServers: transformedServers }; diff --git a/src/features/mcp/factorydroid-mcp.ts b/src/features/mcp/factorydroid-mcp.ts index fa4f19239..5e210e83a 100644 --- a/src/features/mcp/factorydroid-mcp.ts +++ b/src/features/mcp/factorydroid-mcp.ts @@ -59,11 +59,14 @@ export class FactorydroidMcp extends ToolMcp { rulesyncMcp, validate = true, }: ToolMcpFromRulesyncMcpParams): FactorydroidMcp { - const json = rulesyncMcp.getJson(); + // Use getMcpServers() (not getJson()) so rulesync-only fields and + // codex-only fields (`envVars`) are stripped before writing the + // Factory Droid config. + const mcpServers = rulesyncMcp.getMcpServers(); // Factory Droid uses standard MCP format without transformations const factorydroidConfig = { - mcpServers: json.mcpServers || {}, + mcpServers, }; const fileContent = JSON.stringify(factorydroidConfig, null, 2); diff --git a/src/features/mcp/geminicli-mcp.test.ts b/src/features/mcp/geminicli-mcp.test.ts index 615222e16..eb1834bdf 100644 --- a/src/features/mcp/geminicli-mcp.test.ts +++ b/src/features/mcp/geminicli-mcp.test.ts @@ -399,6 +399,41 @@ describe("GeminiCliMcp", () => { }); describe("fromRulesyncMcp", () => { + it("should strip codex-only envVars from gemini output", async () => { + // Regression test: prior to migrating fromRulesyncMcp to + // `getMcpServers()`, the gemini generator read mcpServers from + // `rulesyncMcp.getJson()` (unfiltered), causing codex-only fields like + // `envVars` to leak into ~/.gemini/settings.json. Strip must apply + // here so the field is absent from gemini output. + const jsonData = { + mcpServers: { + pal: { + type: "stdio", + command: "uvx", + args: ["pal-mcp-server"], + envVars: ["OPENAI_API_KEY"], + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const geminiCliMcp = await GeminiCliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + }); + + const json = geminiCliMcp.getJson() as any; + expect(json.mcpServers.pal).toBeDefined(); + expect(json.mcpServers.pal.envVars).toBeUndefined(); + // Defensive: other fields survive. + expect(json.mcpServers.pal.command).toBe("uvx"); + expect(json.mcpServers.pal.args).toEqual(["pal-mcp-server"]); + }); + it("should create instance from RulesyncMcp with default parameters", async () => { const jsonData = { mcpServers: { diff --git a/src/features/mcp/geminicli-mcp.ts b/src/features/mcp/geminicli-mcp.ts index 65ed5ebf2..bf46c129a 100644 --- a/src/features/mcp/geminicli-mcp.ts +++ b/src/features/mcp/geminicli-mcp.ts @@ -72,7 +72,10 @@ export class GeminiCliMcp extends ToolMcp { JSON.stringify({ mcpServers: {} }, null, 2), ); const json = JSON.parse(fileContent); - const newJson = { ...json, mcpServers: rulesyncMcp.getJson().mcpServers }; + // Use getMcpServers() (not getJson()) so rulesync-only fields and + // codex-only fields (`envVars`) are stripped before writing the + // gemini settings file. + const newJson = { ...json, mcpServers: rulesyncMcp.getMcpServers() }; return new GeminiCliMcp({ outputRoot, diff --git a/src/features/mcp/junie-mcp.ts b/src/features/mcp/junie-mcp.ts index f58ab5b3a..3a4a6e712 100644 --- a/src/features/mcp/junie-mcp.ts +++ b/src/features/mcp/junie-mcp.ts @@ -57,11 +57,22 @@ export class JunieMcp extends ToolMcp { rulesyncMcp, validate = true, }: ToolMcpFromRulesyncMcpParams): JunieMcp { + // Preserve top-level fields ($schema, etc.) from the source JSON, but + // use getMcpServers() (not getJson().mcpServers) so rulesync-only + // fields and codex-only fields (`envVars`) are stripped before + // writing the junie config. + const json = rulesyncMcp.getJson(); + const fileContent = JSON.stringify( + { ...json, mcpServers: rulesyncMcp.getMcpServers() }, + null, + 2, + ); + return new JunieMcp({ outputRoot, relativeDirPath: this.getSettablePaths().relativeDirPath, relativeFilePath: this.getSettablePaths().relativeFilePath, - fileContent: rulesyncMcp.getFileContent(), + fileContent, validate, }); } diff --git a/src/features/mcp/rovodev-mcp.ts b/src/features/mcp/rovodev-mcp.ts index b60043c45..3fa7589a8 100644 --- a/src/features/mcp/rovodev-mcp.ts +++ b/src/features/mcp/rovodev-mcp.ts @@ -114,8 +114,10 @@ export class RovodevMcp extends ToolMcp { ); const json = parseRovodevMcpJson(fileContent, paths.relativeDirPath, paths.relativeFilePath); - const rulesyncJson = rulesyncMcp.getJson(); - const mcpServers = isMcpServers(rulesyncJson.mcpServers) ? rulesyncJson.mcpServers : {}; + // Use getMcpServers() (not getJson()) so rulesync-only fields and + // codex-only fields (`envVars`) are stripped before writing the + // rovodev config. + const mcpServers = rulesyncMcp.getMcpServers(); const rovodevConfig = { ...json, mcpServers }; diff --git a/src/features/mcp/rulesync-mcp.test.ts b/src/features/mcp/rulesync-mcp.test.ts index 1c8b012de..11198d912 100644 --- a/src/features/mcp/rulesync-mcp.test.ts +++ b/src/features/mcp/rulesync-mcp.test.ts @@ -987,10 +987,10 @@ describe("RulesyncMcp", () => { }); describe("getMcpServers field stripping", () => { - it("should strip codex-specific env_vars from getMcpServers output", () => { - // env_vars is codex-only; it must NOT leak into other tools' generated + it("should strip codex-specific envVars from getMcpServers output", () => { + // envVars is codex-only; it must NOT leak into other tools' generated // configs (claudecode, opencode, kilo, geminicli, etc.) which all - // consume getMcpServers(). The codex generator reads env_vars directly + // consume getMcpServers(). The codex generator reads envVars directly // from getJson() instead. const rulesyncMcp = new RulesyncMcp({ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, @@ -1001,7 +1001,7 @@ describe("RulesyncMcp", () => { type: "stdio", command: "uvx", args: ["pal-mcp-server"], - env_vars: ["OPENAI_API_KEY", "OPENROUTER_API_KEY"], + envVars: ["OPENAI_API_KEY", "OPENROUTER_API_KEY"], }, }, }), @@ -1011,10 +1011,10 @@ describe("RulesyncMcp", () => { expect(servers.pal).toBeDefined(); expect((servers.pal as any).command).toBe("uvx"); - expect((servers.pal as any).env_vars).toBeUndefined(); + expect((servers.pal as any).envVars).toBeUndefined(); }); - it("should still expose env_vars via getJson() for the codex generator", () => { + it("should still expose envVars via getJson() for the codex generator", () => { const rulesyncMcp = new RulesyncMcp({ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, relativeFilePath: "mcp.json", @@ -1023,14 +1023,14 @@ describe("RulesyncMcp", () => { pal: { type: "stdio", command: "uvx", - env_vars: ["OPENAI_API_KEY"], + envVars: ["OPENAI_API_KEY"], }, }, }), }); const fromJson = rulesyncMcp.getJson().mcpServers.pal; - expect((fromJson as any).env_vars).toEqual(["OPENAI_API_KEY"]); + expect((fromJson as any).envVars).toEqual(["OPENAI_API_KEY"]); }); }); diff --git a/src/features/mcp/rulesync-mcp.ts b/src/features/mcp/rulesync-mcp.ts index 8ea1bad6f..e62849ac3 100644 --- a/src/features/mcp/rulesync-mcp.ts +++ b/src/features/mcp/rulesync-mcp.ts @@ -141,14 +141,19 @@ export class RulesyncMcp extends RulesyncFile { } getMcpServers(): McpServers { - const entries = Object.entries(this.json.mcpServers); + // Tolerate missing/empty mcpServers (e.g., a RulesyncMcp constructed + // from `{}` with validation disabled). Callers that previously read + // `getJson().mcpServers` via the `isMcpServers` guard relied on this + // resilience. + const mcpServers = this.json.mcpServers ?? {}; + const entries = Object.entries(mcpServers); return Object.fromEntries( entries.map(([serverName, serverConfig]) => { - // `env_vars` is codex-specific: the codex generator reads it directly + // `envVars` is codex-specific: the codex generator reads it directly // from the unfiltered source JSON. Strip here so it does not leak // into other tools' outputs. - return [serverName, omit(serverConfig, ["targets", "description", "exposed", "env_vars"])]; + return [serverName, omit(serverConfig, ["targets", "description", "exposed", "envVars"])]; }), ); } diff --git a/src/types/mcp.ts b/src/types/mcp.ts index 0fb9732fa..bfbad2c82 100644 --- a/src/types/mcp.ts +++ b/src/types/mcp.ts @@ -18,10 +18,15 @@ export const McpServerSchema = z.looseObject({ kiroAutoApprove: z.optional(z.array(z.string())), kiroAutoBlock: z.optional(z.array(z.string())), // Codex CLI-specific: list of shell env var names that codex should pass - // through from the user's environment to the MCP server process. Only - // honoured by the codex generator; stripped by `RulesyncMcp.getMcpServers()` - // so it does not leak into other tools' configs. - env_vars: z.optional(z.array(z.string())), + // through from the user's environment to the MCP server process. + // Distinct from `env` (a literal name→value map): `envVars` is a list of + // variable NAMES whose values come from the user's shell at runtime. + // Only honoured by the codex generator (renamed to `env_vars` in codex + // TOML output, matching codex's native field name — see the + // `enabledTools`→`enabled_tools` precedent in `codexcli-mcp.ts`). + // Stripped by `RulesyncMcp.getMcpServers()` so it does not leak into + // other tools' configs. + envVars: z.optional(z.array(z.string())), headers: z.optional(z.record(z.string(), z.string())), enabledTools: z.optional(z.array(z.string())), disabledTools: z.optional(z.array(z.string())),