fix(mcp): isolate ClaudeRegistrar CLI config env (#1888)

## Description

`ClaudeRegistrar` accepts `home_dir` and `config_dir` overrides so
isolated callers can keep Claude config reads and file fallback writes
away from the real user profile. The CLI path did not carry that
resolved config location into the `claude` subprocess, so `claude mcp
add` and `claude mcp remove` could still inherit the caller's real
Claude environment while the registrar's file paths pointed somewhere
else.

This changes the CLI-backed register and unregister paths to pass a
narrow `CLAUDE_CONFIG_DIR` environment only when constructor overrides
request isolation. Normal user sessions keep the ambient subprocess
environment, server `-e KEY=VALUE` arguments remain unchanged, and
isolated registrars make the Claude CLI see the same config directory as
Headroom's file-backed paths. Closes #1861.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- Added a narrow `ClaudeRegistrar` subprocess-env helper that returns an
isolated `CLAUDE_CONFIG_DIR` only when `home_dir` or `config_dir` is
supplied.
- Passed the isolated env into both `claude mcp add` and `claude mcp
remove`.
- Kept server env values in `ServerSpec.env` as existing `claude mcp add
-e KEY=VALUE` arguments.
- Added regression coverage for CLI add and remove with `home_dir`, plus
explicit `config_dir` precedence over an ambient `CLAUDE_CONFIG_DIR`.

## Testing

- [x] Unit tests pass (`uv run pytest
tests/test_mcp_registry/test_claude_registrar.py -q`)
- [x] Linting passes (`uv run ruff check headroom/mcp_registry/claude.py
tests/test_mcp_registry/test_claude_registrar.py`)
- [ ] Type checking passes (`uv run mypy headroom`)
- [x] New tests added for new functionality when applicable
- [x] Manual testing performed

### Test Output

```text
uv run pytest tests/test_mcp_registry/test_claude_registrar.py -q
======================== 26 passed, 1 warning in 0.17s ========================

uv run ruff check headroom/mcp_registry/claude.py tests/test_mcp_registry/test_claude_registrar.py
All checks passed!

uv run ruff format tests/test_mcp_registry/test_claude_registrar.py --check
1 file already formatted
```

## Real Behavior Proof

- Environment: local test runner with mocked Claude CLI subprocess
calls; no real user Claude config touched.
- Exact command / steps: Construct
`ClaudeRegistrar(claude_cli="/usr/local/bin/claude", home_dir=tmp_path)`
and an explicit `config_dir` variant, then exercise
`register_server(...)` and `unregister_server(...)` through the existing
mocked subprocess path.
- Observed result: CLI add and remove calls receive
`env["CLAUDE_CONFIG_DIR"]` matching the registrar's resolved config
directory when isolation is requested. Existing CLI command shape,
server `-e` argument behavior, and file fallback behavior remain intact.
- Not tested: live Claude Code CLI file writes; the PR proves Headroom's
child-process environment handoff without mutating a real Claude
installation.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] I have updated the CHANGELOG.md if applicable

## Additional Notes

Documentation and changelog updates are N/A because this fixes
`ClaudeRegistrar` override isolation rather than adding a user-facing
command or config option. Live Claude CLI config writes are
intentionally left out of local validation to avoid touching real user
configuration.
This commit is contained in:
Rod Boev 2026-07-08 18:36:12 -04:00 committed by GitHub
parent 9d42ebaa1a
commit 1c947b1103
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 48 additions and 7 deletions

View file

@ -47,6 +47,7 @@ class ClaudeRegistrar(MCPRegistrar):
"""
home = home_dir if home_dir is not None else Path.home()
self._claude_dir = _resolve_claude_config_dir(home, config_dir, honor_env=home_dir is None)
self._isolated_cli_env = home_dir is not None or config_dir is not None
self._modern_config = self._claude_dir / ".claude.json"
self._legacy_config = self._claude_dir / "mcp.json"
if claude_cli is ...:
@ -96,6 +97,7 @@ class ClaudeRegistrar(MCPRegistrar):
[str(self._claude_cli), "mcp", "remove", server_name, "-s", "user"],
capture_output=True,
text=True,
env=self._claude_cli_env(),
)
if result.returncode == 0:
return True
@ -121,6 +123,7 @@ class ClaudeRegistrar(MCPRegistrar):
cmd,
capture_output=True,
text=True,
env=self._claude_cli_env(),
)
if result.returncode == 0:
return RegisterResult(RegisterStatus.REGISTERED, "via `claude mcp add` (scope: user)")
@ -187,10 +190,16 @@ class ClaudeRegistrar(MCPRegistrar):
return None
return _entry_to_spec(server_name, entry)
# ----------------------------------------------------------------------
# Helpers
# ----------------------------------------------------------------------
# ----------------------------------------------------------------------
# Helpers
# ----------------------------------------------------------------------
def _claude_cli_env(self) -> dict[str, str] | None:
if not self._isolated_cli_env:
return None
env = os.environ.copy()
env["CLAUDE_CONFIG_DIR"] = str(self._claude_dir)
return env
def _resolve_claude_config_dir(

View file

@ -158,8 +158,9 @@ def test_register_via_cli_calls_claude_mcp_add(
with patch("subprocess.run", return_value=fake_result) as run_mock:
result = reg.register_server(_install_spec(monkeypatch))
assert result.status == RegisterStatus.REGISTERED
cmds = [call.args[0] for call in run_mock.call_args_list]
add_cmd = next(c for c in cmds if "add" in c)
add_call = run_mock.call_args
assert add_call is not None
add_cmd = add_call.args[0]
assert add_cmd[:6] == [
"/usr/local/bin/claude",
"mcp",
@ -173,6 +174,7 @@ def test_register_via_cli_calls_claude_mcp_add(
_RESOLVED_COMMAND[0],
*_RESOLVED_ARGS,
]
assert add_call.kwargs["env"]["CLAUDE_CONFIG_DIR"] == str(tmp_path / ".claude")
def test_register_via_cli_includes_env(tmp_path: Path) -> None:
@ -186,10 +188,38 @@ def test_register_via_cli_includes_env(tmp_path: Path) -> None:
fake_result = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="")
with patch("subprocess.run", return_value=fake_result) as run_mock:
reg.register_server(spec)
add_cmd = next(c for c in [call.args[0] for call in run_mock.call_args_list] if "add" in c)
add_call = run_mock.call_args
assert add_call is not None
add_cmd = add_call.args[0]
assert "-e" in add_cmd
e_idx = add_cmd.index("-e")
assert add_cmd[e_idx + 1] == "HEADROOM_PROXY_URL=http://127.0.0.1:9000"
assert add_call.kwargs["env"]["CLAUDE_CONFIG_DIR"] == str(tmp_path / ".claude")
def test_register_via_cli_without_overrides_keeps_ambient_env(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("CLAUDE_CONFIG_DIR", "ambient")
reg = ClaudeRegistrar(claude_cli="/usr/local/bin/claude")
fake_result = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="")
with patch("subprocess.run", return_value=fake_result) as run_mock:
reg.register_server(_spec())
assert run_mock.call_args is not None
assert run_mock.call_args.kwargs["env"] is None
def test_register_via_cli_prefers_explicit_config_dir_over_ambient_env(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
config_dir = tmp_path / "explicit-config"
monkeypatch.setenv("CLAUDE_CONFIG_DIR", "ambient")
reg = ClaudeRegistrar(claude_cli="/usr/local/bin/claude", config_dir=config_dir)
fake_result = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="")
with patch("subprocess.run", return_value=fake_result) as run_mock:
reg.register_server(_spec())
assert run_mock.call_args is not None
assert run_mock.call_args.kwargs["env"]["CLAUDE_CONFIG_DIR"] == str(config_dir)
def test_register_writes_file_when_no_cli(tmp_path: Path) -> None:
@ -333,9 +363,11 @@ def test_unregister_via_cli(tmp_path: Path) -> None:
ok = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="")
with patch("subprocess.run", return_value=ok) as run_mock:
assert reg.unregister_server("headroom") is True
cmd = run_mock.call_args_list[0].args[0]
assert run_mock.call_args is not None
cmd = run_mock.call_args.args[0]
assert cmd[:5] == ["/usr/local/bin/claude", "mcp", "remove", "headroom", "-s"]
assert cmd[5] == "user"
assert run_mock.call_args.kwargs["env"]["CLAUDE_CONFIG_DIR"] == str(tmp_path / ".claude")
def test_unregister_via_file_when_no_cli(tmp_path: Path) -> None: