Skip to content

Fix: mcp config: readd support for file secrets#2476

Open
malikwirin wants to merge 2 commits intocharmbracelet:mainfrom
malikwirin:fix/2334
Open

Fix: mcp config: readd support for file secrets#2476
malikwirin wants to merge 2 commits intocharmbracelet:mainfrom
malikwirin:fix/2334

Conversation

@malikwirin
Copy link
Copy Markdown

@malikwirin malikwirin commented Mar 24, 2026

Problem

MCP servers that require secrets (e.g. API tokens) currently rely on environment variable expansion via $(echo $VAR) syntax, which spawns a shell process. This makes it cumbersome to use file-based secret management tools like sops, systemd credentials, or Docker secrets.

Closes #2334

Solution

Adds a new env_files field to MCPConfig that maps environment variable names to file paths. Crush reads the file contents at startup and injects them as environment variables when spawning the MCP server process

{
  "mcp": {
    "codeberg": {
      "type": "stdio",
      "command": "/path/to/forgejo-mcp",
      "args": ["transport", "stdio", "--url", "https://codeberg.org"],
      "env_files": {
        "FORGEJO_ACCESS_TOKEN": "/run/secrets/codeberg_token"
      }
    }
  }
}

Changes

  • Adds EnvFiles map[string]string to MCPConfig in config.go
  • Adds resolveEnvFiles() helper that reads file contents and trims trailing newlines
  • Extends MCPConfig.ResolvedEnv() to merge env and env_files
  • Documents the new field in README.md with a usage example
  • Adds tests for parsing, resolution, missing files, and duplicate key behavior

Notes

If the same variable is set in both env and env_files, both entries will be present in the resulting slice. The last-wins behavior of os/exec means env_files takes precedence in practice. This is documented via TestMCPConfig_ResolvedEnv_DuplicateKeyEnvWins.

  • I have read CONTRIBUTING.md.
  • I have created a discussion that was approved by a maintainer (for new features).

Introduces the env_files config key that maps environment variable
names to file paths. README updated with usage example and description.
Adds env_files to MCPConfig, mapping env var names to file paths.
Crush reads the file contents at startup and injects them as
environment variables when spawning the MCP server process
@malikwirin malikwirin requested a review from a team as a code owner March 24, 2026 16:47
@malikwirin malikwirin requested review from andreynering and aymanbagabas and removed request for a team March 24, 2026 16:47
@charmcli
Copy link
Copy Markdown
Contributor

charmcli commented Mar 24, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@malikwirin malikwirin changed the title Fix: mcp config: add support for file secrets Fix: mcp config: readd support for file secrets Mar 24, 2026
@malikwirin
Copy link
Copy Markdown
Author

I have read the Contributor License Agreement (CLA) and hereby sign the CLA.

@malikwirin
Copy link
Copy Markdown
Author

Currently getting the following error in real tests

2026/03/24 19:03:00 ERRO Error reading env file error="open /tmp/test-token: no such file or directory" path=/tmp/test-token source=github.com/charmbracelet/crush/internal/config/config.go:659 variable=MY_TOKEN

@malikwirin malikwirin changed the title Fix: mcp config: readd support for file secrets WIP: Fix: mcp config: readd support for file secrets Mar 24, 2026
@malikwirin malikwirin marked this pull request as draft March 24, 2026 18:15
@malikwirin
Copy link
Copy Markdown
Author

Currently getting the following error in real tests

2026/03/24 19:03:00 ERRO Error reading env file error="open /tmp/test-token: no such file or directory" path=/tmp/test-token source=github.com/charmbracelet/crush/internal/config/config.go:659 variable=MY_TOKEN

No this error happened because my crush config was cached inside .crush.

Everything worked out great. Was validating everything somewhat like this:

mkdir ~/crush-test && cd ~/crush-test
echo "my-test-token" > ~/test-token

cat > print-env.sh <<'EOF'
#!/usr/bin/env bash
env | grep MY_TOKEN >> ~/crush-test/mcp-env.log 2>&1
sleep 10
EOF
chmod +x print-env.sh

cat > crush.json <<'EOF'
{
  "mcp": {
    "test-server": {
      "type": "stdio",
      "command": "/home/malik/crush-test/print-env.sh",
      "env_files": {
        "MY_TOKEN": "/home/malik/test-token"
      }
    }
  }
}
EOF

cat ./mcp-env.log
MY_TOKEN=my-test-token

@malikwirin malikwirin marked this pull request as ready for review March 24, 2026 18:29
@malikwirin malikwirin changed the title WIP: Fix: mcp config: readd support for file secrets Fix: mcp config: readd support for file secrets Mar 24, 2026
Copy link
Copy Markdown
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @malikwirin,

I actually think that the right approach is to actually to restore the shell expansion functionality we had before, and not add env_files.

Do you agree? A separate PR with that would be welcome.

@malikwirin
Copy link
Copy Markdown
Author

Hi @malikwirin,

I actually think that the right approach is to actually to restore the shell expansion functionality we had before, and not add env_files.

Do you agree? A separate PR with that would be welcome.

I think the shell expansion functionality has been dropped for security reasons? Can you maybe point me to the PRs that introduced the regression?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regression(mcp): support file-based secrets in env config (restore shell-expansion behavior)

3 participants