feat(codexcli-mcp): support codex-specific env_vars passthrough - #1623
Merged
Conversation
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.
Self-review of dyoshikawa#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.
Owner
|
@sirmacik Thank you! |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Codex CLI supports a per-server array of shell env var names to inherit when launching the MCP server process. This is distinct from
env:envis a literalname → valuemap;envVarsis a list of names whose values come from the user's environment at runtime.Use case: secrets like
OPENAI_API_KEY,JIRA_PERSONAL_TOKEN, etc. that should NOT be literal-encoded into a committedmcp.json.Before this PR, rulesync had no way to express this in the source schema. Users either lost their entries on every
rulesync generateregen, or added them out-of-band, polluting other tools' configs.Design
Source field:
envVars(camelCase)Matches the MCP schema's existing convention (
enabledTools,disabledTools,kiroAutoApprove,httpUrl,networkTimeout,alwaysAllow,kiroAutoBlock). The codex generator renames it to snake_caseenv_varsat emit time, mirroring the establishedenabledTools→enabled_toolspattern (convertToCodexFormat).Strip is centralised in
getMcpServers()The field is stripped via
RulesyncMcp.getMcpServers()so non-codex generators that go through that accessor never see it. The codex generator reads fromgetJson()(unfiltered) and is the only consumer.Generator audit + leak closure
Six generators were bypassing
getMcpServers()and reading server configs directly fromgetJson()orgetFileContent(). Without the migration in this PR they would have leakedenvVars(and the pre-existing rulesync-only fieldstargets/description/exposed) into their tool outputs:cline-mcp.ts— was usinggetFileContent()cursor-mcp.ts— was usinggetJson()factorydroid-mcp.ts— was usinggetJson()geminicli-mcp.ts— was usinggetJson()junie-mcp.ts— was usinggetFileContent()rovodev-mcp.ts— was usinggetJson()Each is migrated to
getMcpServers().clineandjunierebuild the file content fromgetJson()top-level +getMcpServers()to preserve$schemaand other top-level fields. The cursor "complex configuration" test was updated — it had been asserting the buggy leaked-targetsoutput.getMcpServers()is also hardened to tolerate missingmcpServers(returns{}instead of throwing onObject.entries(undefined)) so callers no longer need the priorisMcpServersguard.Source example
{ "mcpServers": { "pal": { "type": "stdio", "command": "uvx", "args": ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git", "pal-mcp-server"], "envVars": ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] } } }Generated
~/.codex/config.toml:Other tools' outputs (
~/.claude.json,~/.config/opencode/opencode.json,~/.config/kilo/kilo.json,~/.gemini/settings.json,.cursor/mcp.json,.cline/...,.junie/...,.droid/...,~/.rovodev/mcp.json):envVarsstripped, never written.Tests
codexcli: source camelCaseenvVars→ output snake_caseenv_vars; source key absent; defensive assertions on surviving fieldscodexcli: coexistence ofenvVars+envon the same servercodexcli: round-trip codexenv_vars→toRulesyncMcp→envVarsrulesync-mcp:envVarsis stripped fromgetMcpServers()outputrulesync-mcp:envVarssurvives viagetJson()for the codex generatorgeminicli: representative non-codex leak-prevention test —envVarsis absent from gemini outputAll 656 MCP tests pass.
pnpm cicheckclean.Docs
Added new section to
docs/reference/file-formats.md(synced intoskills/rulesync/file-formats.md) covering the source vs codex naming, theenvvsenvVarsdistinction, the full enumeration of non-codex tools where it's stripped, and the precedence-deferral note.Risk
Low — additive change at the schema level. The leak-closure migrations are mechanical (one accessor swap per file). The cursor complex-config test update is the only test change that reflects a behaviour change (correctly stripping
targets); all other changes are additive or strictly cleaner.