Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/crux_cli/cli/commands/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,58 @@ def cmd_mcp_remove(args: argparse.Namespace) -> None:
del reg[section][name]
save_registry(reg)
print(f"\u2705 Removed MCP '{name}' from registry")

# Clean up keychain secrets if the MCP had auth configured
_cleanup_secrets(name, data, args)

print(" Note: run 'crux project sync' to regenerate project configurations")


def _cleanup_secrets(name: str, data: dict[str, Any], args: argparse.Namespace) -> None:
"""Prompt to remove keychain secrets for a removed MCP, respecting CLI flags."""
from crux_cli.secrets import get_backend, load_secrets_index

auth = data.get("auth", {})
env_vars = auth.get("env_vars", [])
if not env_vars:
return

backend = get_backend()
stored = [v for v in env_vars if backend.get(name, v)]
if not stored:
return

keep_secrets = getattr(args, "keep_secrets", False)
remove_secrets = getattr(args, "remove_secrets", False)

if keep_secrets:
should_remove = False
elif remove_secrets:
should_remove = True
elif sys.stdin.isatty():
print(f" \U0001f511 Found {len(stored)} stored secret(s): {', '.join(stored)}")
answer = input(" Also remove secrets from keychain? [Y/n]: ").strip().lower()
should_remove = answer != "n"
else:
# Non-interactive, no flag — default to keeping secrets
print(f" \U0001f511 Found {len(stored)} secret(s) kept (use --remove-secrets to clean up)")
return

if should_remove:
for var in stored:
backend.delete(name, var)
# Clean up any remaining index entries for env vars that weren't stored
index = load_secrets_index()
if name in index:
index.pop(name, None)
from crux_cli.secrets import save_secrets_index

save_secrets_index(index)
print(f" \U0001f5d1\ufe0f Removed {len(stored)} secret(s) from keychain")
else:
print(" Kept secrets in keychain")


def cmd_mcp_list(args: argparse.Namespace) -> None:
"""crux mcp list — show registered MCP servers."""
reg = load_registry()
Expand Down
3 changes: 3 additions & 0 deletions src/crux_cli/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ def main() -> None:

p = mcp_sub.add_parser("remove", help="Unregister an MCP server")
p.add_argument("name", help="Name to remove")
secrets_group = p.add_mutually_exclusive_group()
secrets_group.add_argument("--keep-secrets", action="store_true", help="Keep keychain secrets (no prompt)")
secrets_group.add_argument("--remove-secrets", action="store_true", help="Remove keychain secrets (no prompt)")
p.set_defaults(func=cmd_mcp_remove)

p = mcp_sub.add_parser("list", help="List registered MCP servers")
Expand Down
19 changes: 19 additions & 0 deletions tests/integration/test_cli_mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,25 @@ def test_remove_nonexistent_fails(self, crux_env):
result = run_crux("mcp", "remove", "nope", env=env)
assert result.returncode != 0

def test_remove_with_keep_secrets_flag(self, crux_env):
env, root = crux_env
run_crux("mcp", "add", "authed", "--npx", "@test/authed", "--keychain", "KEY1,KEY2", env=env)
result = run_crux("mcp", "remove", "authed", "--keep-secrets", env=env)
assert result.returncode == 0
assert "Removed" in result.stdout

def test_remove_with_remove_secrets_flag(self, crux_env):
env, root = crux_env
run_crux("mcp", "add", "authed2", "--npx", "@test/authed2", "--keychain", "KEY1", env=env)
result = run_crux("mcp", "remove", "authed2", "--remove-secrets", env=env)
assert result.returncode == 0
assert "Removed" in result.stdout

def test_remove_secrets_flags_mutually_exclusive(self, crux_env):
env, root = crux_env
result = run_crux("mcp", "remove", "x", "--keep-secrets", "--remove-secrets", env=env)
assert result.returncode != 0


@pytest.mark.integration
class TestMcpList:
Expand Down
127 changes: 127 additions & 0 deletions tests/unit/test_mcp_remove_secrets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
"""Tests for secret cleanup during crux mcp remove."""

from __future__ import annotations

import argparse
from unittest.mock import MagicMock, patch

import pytest

from crux_cli.cli.commands.mcp import _cleanup_secrets


@pytest.fixture
def mcp_data_with_secrets():
return {
"auth": {
"type": "keychain",
"env_vars": ["API_KEY", "SECRET_TOKEN"],
}
}


@pytest.fixture
def mcp_data_no_auth():
return {}


class TestCleanupSecrets:
def test_no_auth_block_does_nothing(self, mcp_data_no_auth):
args = argparse.Namespace(keep_secrets=False, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_no_auth, args)

def test_no_env_vars_does_nothing(self):
data = {"auth": {"type": "external-cli"}}
args = argparse.Namespace(keep_secrets=False, remove_secrets=False)
_cleanup_secrets("test-mcp", data, args)

@patch("crux_cli.secrets.get_backend")
def test_no_stored_secrets_does_nothing(self, mock_get_backend, mcp_data_with_secrets):
backend = MagicMock()
backend.get.return_value = None
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=False, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
backend.delete.assert_not_called()

@patch("crux_cli.secrets.save_secrets_index")
@patch("crux_cli.secrets.load_secrets_index", return_value={})
@patch("crux_cli.secrets.get_backend")
def test_remove_secrets_flag_deletes_all(self, mock_get_backend, _mock_idx, _mock_save, mcp_data_with_secrets):
backend = MagicMock()
backend.get.return_value = "stored-value"
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=False, remove_secrets=True)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
assert backend.delete.call_count == 2
backend.delete.assert_any_call("test-mcp", "API_KEY")
backend.delete.assert_any_call("test-mcp", "SECRET_TOKEN")

@patch("crux_cli.secrets.get_backend")
def test_keep_secrets_flag_skips_deletion(self, mock_get_backend, mcp_data_with_secrets, capsys):
backend = MagicMock()
backend.get.return_value = "stored-value"
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=True, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
backend.delete.assert_not_called()
assert "Kept secrets" in capsys.readouterr().out

@patch("crux_cli.secrets.save_secrets_index")
@patch("crux_cli.secrets.load_secrets_index", return_value={"test-mcp": ["API_KEY", "SECRET_TOKEN"]})
@patch("crux_cli.secrets.get_backend")
@patch("builtins.input", return_value="")
@patch("sys.stdin")
def test_interactive_yes_default(
self,
mock_stdin,
_mock_input,
mock_get_backend,
_mock_idx,
_mock_save,
mcp_data_with_secrets,
capsys,
):
mock_stdin.isatty.return_value = True
backend = MagicMock()
backend.get.return_value = "stored-value"
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=False, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
assert backend.delete.call_count == 2
assert "Removed 2 secret(s)" in capsys.readouterr().out

@patch("crux_cli.secrets.get_backend")
@patch("builtins.input", return_value="n")
@patch("sys.stdin")
def test_interactive_no(self, mock_stdin, _mock_input, mock_get_backend, mcp_data_with_secrets, capsys):
mock_stdin.isatty.return_value = True
backend = MagicMock()
backend.get.return_value = "stored-value"
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=False, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
backend.delete.assert_not_called()
assert "Kept secrets" in capsys.readouterr().out

@patch("crux_cli.secrets.get_backend")
@patch("sys.stdin")
def test_non_interactive_keeps_secrets(self, mock_stdin, mock_get_backend, mcp_data_with_secrets, capsys):
mock_stdin.isatty.return_value = False
backend = MagicMock()
backend.get.return_value = "stored-value"
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=False, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
backend.delete.assert_not_called()
assert "kept" in capsys.readouterr().out

@patch("crux_cli.secrets.get_backend")
def test_partially_stored_only_counts_stored(self, mock_get_backend, mcp_data_with_secrets, capsys):
"""Only secrets that are actually stored are counted."""
backend = MagicMock()
backend.get.side_effect = lambda name, key: "val" if key == "API_KEY" else None
mock_get_backend.return_value = backend
args = argparse.Namespace(keep_secrets=True, remove_secrets=False)
_cleanup_secrets("test-mcp", mcp_data_with_secrets, args)
assert "Kept secrets" in capsys.readouterr().out
Loading